Skip to content

Commit

Permalink
pe: simplify generate_hash()
Browse files Browse the repository at this point in the history
Copying the value of datasize_in to two further variables and then using
all three randomly in the code makes it hard to read.

datasize_in is never changed in generate_hash() so we can do with this
parameter alone. Rename it to datasize.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
  • Loading branch information
xypron authored and vathpela committed Sep 10, 2021
1 parent c1a84dc commit 2699836
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 14 deletions.
2 changes: 1 addition & 1 deletion include/pe.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ handle_image (void *data, unsigned int datasize,
UINTN *alloc_pages);

EFI_STATUS
generate_hash (char *data, unsigned int datasize_in,
generate_hash (char *data, unsigned int datasize,
PE_COFF_LOADER_IMAGE_CONTEXT *context,
UINT8 *sha256hash, UINT8 *sha1hash);

Expand Down
23 changes: 10 additions & 13 deletions pe.c
Original file line number Diff line number Diff line change
Expand Up @@ -298,26 +298,22 @@ get_section_vma_by_name (char *name, size_t namesz,
*/

EFI_STATUS
generate_hash(char *data, unsigned int datasize_in,
generate_hash(char *data, unsigned int datasize,
PE_COFF_LOADER_IMAGE_CONTEXT *context, UINT8 *sha256hash,
UINT8 *sha1hash)
{
unsigned int sha256ctxsize, sha1ctxsize;
unsigned int size = datasize_in;
void *sha256ctx = NULL, *sha1ctx = NULL;
char *hashbase;
unsigned int hashsize;
unsigned int SumOfBytesHashed, SumOfSectionBytes;
unsigned int index, pos;
unsigned int datasize;
EFI_IMAGE_SECTION_HEADER *Section;
EFI_IMAGE_SECTION_HEADER *SectionHeader = NULL;
EFI_STATUS efi_status = EFI_SUCCESS;
EFI_IMAGE_DOS_HEADER *DosHdr = (void *)data;
unsigned int PEHdr_offset = 0;

size = datasize = datasize_in;

if (datasize <= sizeof (*DosHdr) ||
DosHdr->e_magic != EFI_IMAGE_DOS_SIGNATURE) {
perror(L"Invalid signature\n");
Expand Down Expand Up @@ -346,7 +342,7 @@ generate_hash(char *data, unsigned int datasize_in,
hashbase = data;
hashsize = (char *)&context->PEHdr->Pe32.OptionalHeader.CheckSum -
hashbase;
check_size(data, datasize_in, hashbase, hashsize);
check_size(data, datasize, hashbase, hashsize);

if (!(Sha256Update(sha256ctx, hashbase, hashsize)) ||
!(Sha1Update(sha1ctx, hashbase, hashsize))) {
Expand All @@ -359,7 +355,7 @@ generate_hash(char *data, unsigned int datasize_in,
hashbase = (char *)&context->PEHdr->Pe32.OptionalHeader.CheckSum +
sizeof (int);
hashsize = (char *)context->SecDir - hashbase;
check_size(data, datasize_in, hashbase, hashsize);
check_size(data, datasize, hashbase, hashsize);

if (!(Sha256Update(sha256ctx, hashbase, hashsize)) ||
!(Sha1Update(sha1ctx, hashbase, hashsize))) {
Expand All @@ -372,12 +368,12 @@ generate_hash(char *data, unsigned int datasize_in,
EFI_IMAGE_DATA_DIRECTORY *dd = context->SecDir + 1;
hashbase = (char *)dd;
hashsize = context->SizeOfHeaders - (unsigned long)((char *)dd - data);
if (hashsize > datasize_in) {
if (hashsize > datasize) {
perror(L"Data Directory size %d is invalid\n", hashsize);
efi_status = EFI_INVALID_PARAMETER;
goto done;
}
check_size(data, datasize_in, hashbase, hashsize);
check_size(data, datasize, hashbase, hashsize);

if (!(Sha256Update(sha256ctx, hashbase, hashsize)) ||
!(Sha1Update(sha1ctx, hashbase, hashsize))) {
Expand Down Expand Up @@ -491,7 +487,8 @@ generate_hash(char *data, unsigned int datasize_in,
continue;
}

hashbase = ImageAddress(data, size, Section->PointerToRawData);
hashbase = ImageAddress(data, datasize,
Section->PointerToRawData);
if (!hashbase) {
perror(L"Malformed section header\n");
efi_status = EFI_INVALID_PARAMETER;
Expand All @@ -506,7 +503,7 @@ generate_hash(char *data, unsigned int datasize_in,
goto done;
}
hashsize = (unsigned int) Section->SizeOfRawData;
check_size(data, datasize_in, hashbase, hashsize);
check_size(data, datasize, hashbase, hashsize);

if (!(Sha256Update(sha256ctx, hashbase, hashsize)) ||
!(Sha1Update(sha1ctx, hashbase, hashsize))) {
Expand All @@ -532,7 +529,7 @@ generate_hash(char *data, unsigned int datasize_in,
efi_status = EFI_INVALID_PARAMETER;
goto done;
}
check_size(data, datasize_in, hashbase, hashsize);
check_size(data, datasize, hashbase, hashsize);

if (!(Sha256Update(sha256ctx, hashbase, hashsize)) ||
!(Sha1Update(sha1ctx, hashbase, hashsize))) {
Expand All @@ -552,7 +549,7 @@ generate_hash(char *data, unsigned int datasize_in,
hashbase = data + SumOfBytesHashed;
hashsize = datasize - SumOfBytesHashed;

check_size(data, datasize_in, hashbase, hashsize);
check_size(data, datasize, hashbase, hashsize);

if (!(Sha256Update(sha256ctx, hashbase, hashsize)) ||
!(Sha1Update(sha1ctx, hashbase, hashsize))) {
Expand Down

0 comments on commit 2699836

Please sign in to comment.