shim: ensure generate_hash() never operates on a negative (signed) number.

Covscan noticed:
746static EFI_STATUS generate_hash (char *data, unsigned int datasize_in,
747                                 PE_COFF_LOADER_IMAGE_CONTEXT *context,
748                                 UINT8 *sha256hash, UINT8 *sha1hash)
749
750{
...
764
    CID 182849 (#1 of 1): Unsigned compared against 0
    (NO_EFFECT)unsigned_compare: This less-than-zero comparison of an
    unsigned value is never true. datasize_in < 0U.
765        if (datasize_in < 0) {
766                perror(L"Invalid data size\n");
767                return EFI_INVALID_PARAMETER;
768        }

And I guess that's a fair point, but some of the callers take the size
as a signed integer.  So we should be handling that on all the input
cases instead of getting that far.

Signed-off-by: Peter Jones <pjones@redhat.com>
This commit is contained in:
Peter Jones 2017-09-27 16:23:14 -04:00 committed by Peter Jones
parent 2fa2ec8c44
commit db2f5cf15d

34
shim.c
View File

@ -754,11 +754,7 @@ static EFI_STATUS generate_hash (char *data, unsigned int datasize_in,
EFI_IMAGE_DOS_HEADER *DosHdr = (void *)data;
unsigned int PEHdr_offset = 0;
if (datasize_in < 0) {
perror(L"Invalid data size\n");
return EFI_INVALID_PARAMETER;
}
size = datasize = (unsigned int)datasize_in;
size = datasize = datasize_in;
if (datasize <= sizeof (*DosHdr) ||
DosHdr->e_magic != EFI_IMAGE_DOS_SIGNATURE) {
@ -1016,6 +1012,9 @@ static EFI_STATUS verify_buffer (char *data, int datasize,
WIN_CERTIFICATE_EFI_PKCS *cert = NULL;
unsigned int size = datasize;
if (datasize < 0)
return EFI_INVALID_PARAMETER;
if (context->SecDir->Size != 0) {
if (context->SecDir->Size >= size) {
perror(L"Certificate Database size is too large\n");
@ -1801,6 +1800,9 @@ EFI_STATUS shim_verify (void *buffer, UINT32 size)
UINT8 sha1hash[SHA1_DIGEST_SIZE];
UINT8 sha256hash[SHA256_DIGEST_SIZE];
if ((INT32)size < 0)
return EFI_INVALID_PARAMETER;
loader_is_participating = 1;
in_protocol = 1;
@ -1837,6 +1839,9 @@ static EFI_STATUS shim_hash (char *data, int datasize,
{
EFI_STATUS status;
if (datasize < 0)
return EFI_INVALID_PARAMETER;
in_protocol = 1;
status = generate_hash(data, datasize, context, sha256hash, sha1hash);
in_protocol = 0;
@ -1899,17 +1904,20 @@ EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath)
efi_status = FetchNetbootimage(image_handle, &sourcebuffer,
&sourcesize);
if (efi_status != EFI_SUCCESS) {
perror(L"Unable to fetch TFTP image: %r\n", efi_status);
perror(L"Unable to fetch TFTP image: %r\n",
efi_status);
return efi_status;
}
data = sourcebuffer;
datasize = sourcesize;
#if defined(ENABLE_HTTPBOOT)
} else if (find_httpboot(li->DeviceHandle)) {
efi_status = httpboot_fetch_buffer (image_handle, &sourcebuffer,
efi_status = httpboot_fetch_buffer (image_handle,
&sourcebuffer,
&sourcesize);
if (efi_status != EFI_SUCCESS) {
perror(L"Unable to fetch HTTP image: %r\n", efi_status);
perror(L"Unable to fetch HTTP image: %r\n",
efi_status);
return efi_status;
}
data = sourcebuffer;
@ -1920,15 +1928,20 @@ EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath)
* Read the new executable off disk
*/
efi_status = load_image(li, &data, &datasize, PathName);
if (efi_status != EFI_SUCCESS) {
perror(L"Failed to load image %s: %r\n", PathName, efi_status);
perror(L"Failed to load image %s: %r\n",
PathName, efi_status);
PrintErrors();
ClearErrors();
goto done;
}
}
if (datasize < 0) {
efi_status = EFI_INVALID_PARAMETER;
goto done;
}
/*
* We need to modify the loaded image protocol entry before running
* the new binary, so back it up
@ -1939,7 +1952,6 @@ EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath)
* Verify and, if appropriate, relocate and execute the executable
*/
efi_status = handle_image(data, datasize, li);
if (efi_status != EFI_SUCCESS) {
perror(L"Failed to load image: %r\n", efi_status);
PrintErrors();