fallback: read_file(): limit how big the file can be and still be valid

Covscan says:

146        UINTN len = 0;
147        CHAR16 *b = NULL;
   2. tainted_data_argument: Calling function get_file_size taints argument len.
148        rc = get_file_size(fh2, &len);
   3. Condition (INTN)rc < 0, taking false branch.
149        if (EFI_ERROR(rc)) {
150                uefi_call_wrapper(fh2->Close, 1, fh2);
151                return rc;
152        }
153
   4. overflow_assign: Assigning overflowed or truncated value (or a value computed from an overflowed or a truncated value) to b.
   8. overflow: Add operation overflows on operands len and 2UL. Example value for operand: len = 18446744073709551614.
154        b = AllocateZeroPool(len + 2);

Technically we can't handle a file larger than 0xfffffffffffffffd (on
x86_64) because when we try to allocate the buffer to hold it with a
trailing UCS-2 NUL we overflow to 0.  Also our filesystem can't hold a
file bigger than 4GB...  So this is probably actually broken on 32-bit
platforms.

This patch limits it to some handy amount like 1024 * PAGE_SIZE, aka
4MB.

Note that this doesn't appear to be exploitable (at least on edk2-based
firmwares), because AllocateZeroPool() has a minimum granularity of 1
page, so even if you overflow it with a 4GB file, we'll get 1 page out
of it and then try to read 1 byte into it, and then it's just going to
be a parse error on the CSV.  Even if we error on the sentinal UCS-2 NUL
we put at the end, it'll still be inside of the zeroed page, and it still
won't fault or overwrite any meaningful data.

Signed-off-by: Peter Jones <pjones@redhat.com>
This commit is contained in:
Peter Jones 2017-09-27 13:15:13 -04:00 committed by Peter Jones
parent 05458d227f
commit 809dc7a18b

View File

@ -149,6 +149,11 @@ read_file(EFI_FILE_HANDLE fh, CHAR16 *fullpath, CHAR16 **buffer, UINT64 *bs)
return rc;
}
if (len > 1024 * PAGE_SIZE) {
uefi_call_wrapper(fh2->Close, 1, fh2);
return EFI_BAD_BUFFER_SIZE;
}
b = AllocateZeroPool(len + 2);
if (!buffer) {
Print(L"Could not allocate memory\n");