From: Ana Bobillo <[email protected]>
efi_image_parse() collects the regions of an image to be hashed for
Secure Boot and for TCG2 measurements. It takes the header size, the
section table, each section's raw data and the certificate table from
the image without checking them against the image size:
- SizeOfHeaders and each section's PointerToRawData and SizeOfRawData
are added as regions even when they run past the end of the image,
which is then read out of bounds when the regions are hashed;
- the section table is read wherever SizeOfOptionalHeader and
NumberOfSections put it;
- the end of the certificate table is computed in 32 bits, so a large
offset wraps around and passes the check;
- errors of efi_image_region_add() are ignored.
All of this happens before the signature is verified.
Call efi_check_pe() first, check each offset and size against the image
size without wrapping, add regions only through a checked helper and fail
on its errors. Require the optional header to hold exactly the data
directories it declares, at most 16, as EDK2 and shim do. Take a file
alignment of 0 as the PECOFF default of 512 and require any other to be
a power of two. The padding of a section to the file alignment is hashed
only as far as the image goes, so an image whose last section is not
padded is still accepted. Check the certificate table against the size
of WIN_CERTIFICATE, not of a pointer to it. The sorted section table is
now freed on the error path too, which the new error exits from the
section loop need.
barebox, whose EFI loader is based on this file, added similar checks in
4ec1f7166778 ("efi: validate PE parser input ranges").
Fixes: 4540dabdcaca ("efi_loader: image_loader: support image authentication")
Signed-off-by: Ana Bobillo <[email protected]>
---
lib/efi_loader/efi_image_loader.c | 254 +++++++++++++++++++++---------
1 file changed, 176 insertions(+), 78 deletions(-)
diff --git a/lib/efi_loader/efi_image_loader.c
b/lib/efi_loader/efi_image_loader.c
index 7cda338882c..82083cb73d9 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -393,6 +393,45 @@ void *efi_prepare_aligned_image(void *efi, u64 *efi_size)
}
}
+/**
+ * pe_range_ok() - check that a range lies within the image
+ * @len: size of the image
+ * @off: offset of the range from the start of the image
+ * @size: size of the range
+ *
+ * Return: true if [@off, @off + @size) lies within [0, @len)
+ */
+static bool pe_range_ok(size_t len, size_t off, size_t size)
+{
+ return off <= len && size <= len - off;
+}
+
+/**
+ * efi_image_region_add_checked() - add a region of an image
+ * @regs: pointer to array of regions
+ * @efi: pointer to the image
+ * @len: size of the image
+ * @start: offset of the region in the image
+ * @size: size of the region
+ * @nocheck: flag against overlapped regions
+ *
+ * Add the region [@start, @start + @size[ of the image to @regs if it lies
+ * within the image.
+ *
+ * Return: status code
+ */
+static efi_status_t efi_image_region_add_checked(struct efi_image_regions
*regs,
+ const void *efi, size_t len,
+ size_t start, size_t size,
+ int nocheck)
+{
+ if (!pe_range_ok(len, start, size))
+ return EFI_INVALID_PARAMETER;
+
+ return efi_image_region_add(regs, efi + start, efi + start + size,
+ nocheck);
+}
+
/**
* efi_image_parse() - parse a PE image
* @efi: Pointer to image
@@ -401,8 +440,8 @@ void *efi_prepare_aligned_image(void *efi, u64 *efi_size)
* @auth: Pointer to a pointer to authentication data in PE
* @auth_len: Size of @auth
*
- * Parse image binary in PE32(+) format, assuming that sanity of PE image
- * has been checked by a caller.
+ * Parse image binary in PE32(+) format. Each offset and size taken from the
+ * image is checked against @len before it is used.
* On success, an address of authentication data in @efi and its size will
* be returned in @auth and @auth_len, respectively.
*
@@ -411,97 +450,142 @@ void *efi_prepare_aligned_image(void *efi, u64 *efi_size)
bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
WIN_CERTIFICATE **auth, size_t *auth_len)
{
- struct efi_image_regions *regs;
- IMAGE_DOS_HEADER *dos;
+ struct efi_image_regions *regs = NULL;
IMAGE_NT_HEADERS32 *nt;
- IMAGE_SECTION_HEADER *sections, **sorted;
+ IMAGE_SECTION_HEADER *sections, **sorted = NULL;
int num_regions, num_sections, i;
int ctidx = IMAGE_DIRECTORY_ENTRY_SECURITY;
- u32 align, size, authsz, authoff;
- size_t bytes_hashed;
+ u32 align, authsz, authoff, header_size, num_dirs;
+ size_t bytes_hashed, opt_off, opt_size, dirs_start;
+ size_t csum_off, subsys_off, dir_off, dir_end;
+ size_t sections_off, sections_size;
- dos = (void *)efi;
- nt = (void *)(efi + dos->e_lfanew);
authoff = 0;
authsz = 0;
/*
- * Count maximum number of regions to be digested.
- * We don't have to have an exact number here.
- * See efi_image_region_add()'s in parsing below.
+ * efi_check_pe() checks the DOS and PE signatures and that the image
+ * is large enough to hold a 64bit NT header, which is longer than a
+ * 32bit one. Every optional header field and data directory entry
+ * read below therefore lies within the image.
*/
- num_regions = 3; /* for header */
- num_regions += nt->FileHeader.NumberOfSections;
- num_regions++; /* for extra */
-
- regs = calloc(sizeof(*regs) + sizeof(struct image_region) * num_regions,
- 1);
- if (!regs)
- goto err;
- regs->max = num_regions;
+ if (efi_check_pe(efi, len, (void **)&nt) != EFI_SUCCESS)
+ return false;
+ opt_off = (u8 *)&nt->OptionalHeader - (u8 *)efi;
+ opt_size = nt->FileHeader.SizeOfOptionalHeader;
- /*
- * Collect data regions for hash calculation
- * 1. File headers
- */
if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
IMAGE_NT_HEADERS64 *nt64 = (void *)nt;
IMAGE_OPTIONAL_HEADER64 *opt = &nt64->OptionalHeader;
- /* Skip CheckSum */
- efi_image_region_add(regs, efi, &opt->CheckSum, 0);
- if (nt64->OptionalHeader.NumberOfRvaAndSizes <= ctidx) {
- efi_image_region_add(regs,
- &opt->Subsystem,
- efi + opt->SizeOfHeaders, 0);
- } else {
- /* Skip Certificates Table */
- efi_image_region_add(regs,
- &opt->Subsystem,
- &opt->DataDirectory[ctidx], 0);
- efi_image_region_add(regs,
- &opt->DataDirectory[ctidx] + 1,
- efi + opt->SizeOfHeaders, 0);
-
+ dirs_start = offsetof(IMAGE_OPTIONAL_HEADER64, DataDirectory);
+ csum_off = opt_off + offsetof(IMAGE_OPTIONAL_HEADER64,
+ CheckSum);
+ subsys_off = opt_off + offsetof(IMAGE_OPTIONAL_HEADER64,
+ Subsystem);
+ dir_off = opt_off + offsetof(IMAGE_OPTIONAL_HEADER64,
+ DataDirectory[ctidx]);
+ header_size = opt->SizeOfHeaders;
+ align = opt->FileAlignment;
+ num_dirs = opt->NumberOfRvaAndSizes;
+ if (num_dirs > ctidx) {
authoff = opt->DataDirectory[ctidx].VirtualAddress;
authsz = opt->DataDirectory[ctidx].Size;
}
-
- bytes_hashed = opt->SizeOfHeaders;
- align = opt->FileAlignment;
} else if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
IMAGE_OPTIONAL_HEADER32 *opt = &nt->OptionalHeader;
- /* Skip CheckSum */
- efi_image_region_add(regs, efi, &opt->CheckSum, 0);
- if (nt->OptionalHeader.NumberOfRvaAndSizes <= ctidx) {
- efi_image_region_add(regs,
- &opt->Subsystem,
- efi + opt->SizeOfHeaders, 0);
- } else {
- /* Skip Certificates Table */
- efi_image_region_add(regs, &opt->Subsystem,
- &opt->DataDirectory[ctidx], 0);
- efi_image_region_add(regs,
- &opt->DataDirectory[ctidx] + 1,
- efi + opt->SizeOfHeaders, 0);
-
+ dirs_start = offsetof(IMAGE_OPTIONAL_HEADER32, DataDirectory);
+ csum_off = opt_off + offsetof(IMAGE_OPTIONAL_HEADER32,
+ CheckSum);
+ subsys_off = opt_off + offsetof(IMAGE_OPTIONAL_HEADER32,
+ Subsystem);
+ dir_off = opt_off + offsetof(IMAGE_OPTIONAL_HEADER32,
+ DataDirectory[ctidx]);
+ header_size = opt->SizeOfHeaders;
+ align = opt->FileAlignment;
+ num_dirs = opt->NumberOfRvaAndSizes;
+ if (num_dirs > ctidx) {
authoff = opt->DataDirectory[ctidx].VirtualAddress;
authsz = opt->DataDirectory[ctidx].Size;
}
-
- bytes_hashed = opt->SizeOfHeaders;
- align = opt->FileAlignment;
} else {
log_err("%s: Invalid optional header magic %x\n", __func__,
nt->OptionalHeader.Magic);
+ return false;
+ }
+ dir_end = dir_off + sizeof(IMAGE_DATA_DIRECTORY);
+
+ /*
+ * The optional header must hold exactly the data directories it
+ * declares, at most 16, as EDK2 and shim require. The section table
+ * follows the optional header; it and the whole header area
+ * (SizeOfHeaders) must lie within the image, as must the certificate
+ * table.
+ */
+ num_sections = nt->FileHeader.NumberOfSections;
+ sections_off = opt_off + opt_size;
+ sections_size = (size_t)num_sections * sizeof(IMAGE_SECTION_HEADER);
+ if (num_dirs > IMAGE_NUMBEROF_DIRECTORY_ENTRIES ||
+ opt_size != dirs_start + num_dirs * sizeof(IMAGE_DATA_DIRECTORY) ||
+ !pe_range_ok(len, sections_off, sections_size) ||
+ !pe_range_ok(len, 0, header_size) ||
+ header_size < sections_off + sections_size) {
+ log_err("%s: Invalid PE headers\n", __func__);
+ return false;
+ }
+ if (authsz && !pe_range_ok(len, authoff, authsz)) {
+ log_err("%s: Size for auth too large: %u\n", __func__, authsz);
+ return false;
+ }
+
+ /* The file alignment is a power of two, by default 512 (PECOFF) */
+ if (!align)
+ align = 512;
+ if (align & (align - 1)) {
+ log_err("%s: Invalid file alignment %u\n", __func__, align);
+ return false;
+ }
+
+ /*
+ * Count maximum number of regions to be digested: three for the
+ * header, one for each section and one for extra data.
+ * We don't have to have an exact number here.
+ * See efi_image_region_add()'s in parsing below.
+ */
+ num_regions = 4 + num_sections;
+
+ regs = calloc(sizeof(*regs) + sizeof(struct image_region) * num_regions,
+ 1);
+ if (!regs)
goto err;
+ regs->max = num_regions;
+
+ /*
+ * Collect data regions for hash calculation
+ * 1. File headers
+ */
+ /* Skip CheckSum */
+ if (efi_image_region_add_checked(regs, efi, len, 0, csum_off, 0))
+ goto err;
+ if (num_dirs <= ctidx) {
+ if (efi_image_region_add_checked(regs, efi, len, subsys_off,
+ header_size - subsys_off, 0))
+ goto err;
+ } else {
+ /* Skip Certificates Table */
+ if (efi_image_region_add_checked(regs, efi, len, subsys_off,
+ dir_off - subsys_off, 0))
+ goto err;
+ if (efi_image_region_add_checked(regs, efi, len, dir_end,
+ header_size - dir_end, 0))
+ goto err;
}
+ bytes_hashed = header_size;
+
/* 2. Sections */
- num_sections = nt->FileHeader.NumberOfSections;
- sections = (void *)((uint8_t *)&nt->OptionalHeader +
- nt->FileHeader.SizeOfOptionalHeader);
+ sections = (void *)((uint8_t *)efi + sections_off);
sorted = calloc(sizeof(IMAGE_SECTION_HEADER *), num_sections);
if (!sorted) {
log_err("%s: Out of memory\n", __func__);
@@ -516,14 +600,29 @@ bool efi_image_parse(void *efi, size_t len, struct
efi_image_regions **regp,
qsort(sorted, num_sections, sizeof(sorted[0]), cmp_pe_section);
for (i = 0; i < num_sections; i++) {
+ size_t size;
+
if (!sorted[i]->SizeOfRawData)
continue;
- size = (sorted[i]->SizeOfRawData + align - 1) & ~(align - 1);
- efi_image_region_add(regs, efi + sorted[i]->PointerToRawData,
- efi + sorted[i]->PointerToRawData + size,
- 0);
- log_debug("section[%d](%s): raw: 0x%x-0x%x, virt: %x-%x\n",
+ if (!pe_range_ok(len, sorted[i]->PointerToRawData,
+ sorted[i]->SizeOfRawData)) {
+ log_err("%s: Invalid section %d\n", __func__, i);
+ goto err;
+ }
+ /*
+ * The padding to the file alignment ends with the image. Round
+ * up in 64 bits, where the sum of two u32 cannot wrap around.
+ */
+ size = min_t(u64, ALIGN((u64)sorted[i]->SizeOfRawData, align),
+ len - sorted[i]->PointerToRawData);
+ if (efi_image_region_add_checked(regs, efi, len,
+ sorted[i]->PointerToRawData,
+ size, 0)) {
+ log_err("%s: Invalid section %d\n", __func__, i);
+ goto err;
+ }
+ log_debug("section[%d](%s): raw: 0x%x-0x%zx, virt: %x-%x\n",
i, sorted[i]->Name,
sorted[i]->PointerToRawData,
sorted[i]->PointerToRawData + size,
@@ -534,25 +633,23 @@ bool efi_image_parse(void *efi, size_t len, struct
efi_image_regions **regp,
bytes_hashed += size;
}
free(sorted);
+ sorted = NULL;
/* 3. Extra data excluding Certificates Table */
- if (bytes_hashed + authsz < len) {
+ if (bytes_hashed < len && authsz < len - bytes_hashed) {
log_debug("extra data for hash: %zu\n",
len - (bytes_hashed + authsz));
- efi_image_region_add(regs, efi + bytes_hashed,
- efi + len - authsz, 0);
+ if (efi_image_region_add_checked(regs, efi, len, bytes_hashed,
+ len - bytes_hashed - authsz,
+ 0))
+ goto err;
}
/* Return Certificates Table */
if (authsz) {
- if (len < authoff + authsz) {
- log_err("%s: Size for auth too large: %u >= %zu\n",
- __func__, authsz, len - authoff);
- goto err;
- }
- if (authsz < sizeof(*auth)) {
+ if (authsz < sizeof(**auth)) {
log_err("%s: Size for auth too small: %u < %zu\n",
- __func__, authsz, sizeof(*auth));
+ __func__, authsz, sizeof(**auth));
goto err;
}
*auth = efi + authoff;
@@ -569,6 +666,7 @@ bool efi_image_parse(void *efi, size_t len, struct
efi_image_regions **regp,
return true;
err:
+ free(sorted);
free(regs);
return false;
