From 9d30a941cce5ed055da18398f4deba18830d00d6 Mon Sep 17 00:00:00 2001 From: Asherah Connor Date: Tue, 9 Feb 2021 06:19:48 +0000 Subject: efi_loader: don't load beyond VirtualSize PE section table entries' SizeOfRawData must be a multiple of FileAlignment, and thus may be rounded up and larger than their VirtualSize. We should not load beyond the VirtualSize, which is "the total size of the section when loaded into memory" -- we may clobber real data at the target in some other section, since we load sections in reverse order and sections are usually laid out sequentially. Signed-off-by: Asherah Connor Reviewed-by: Heinrich Schuchardt --- lib/efi_loader/efi_image_loader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index d4dd9e94339..f53ef367ec1 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -843,7 +843,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, sec->Misc.VirtualSize); memcpy(efi_reloc + sec->VirtualAddress, efi + sec->PointerToRawData, - sec->SizeOfRawData); + min(sec->Misc.VirtualSize, sec->SizeOfRawData)); } /* Run through relocations */ -- cgit v1.2.3 From 841f7a4ebbe76c7843a864a5f9ca8f5f95a23df8 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 9 Feb 2021 17:45:33 +0100 Subject: efi_loader: '.' and '..' are directories '.' and '..' are directories. So when looking for capsule files it is sufficient to check that the attribute EFI_FILE_DIRECTORY is not set. We don't have to check for these special names. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_capsule.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 0d5a7b63ec8..d39d7310804 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -753,9 +753,7 @@ static efi_status_t efi_capsule_scan_dir(u16 ***files, unsigned int *num) if (!tmp_size) break; - if (!(dirent->attribute & EFI_FILE_DIRECTORY) && - u16_strcmp(dirent->file_name, L".") && - u16_strcmp(dirent->file_name, L"..")) + if (!(dirent->attribute & EFI_FILE_DIRECTORY)) count++; } -- cgit v1.2.3 From 15bbcafab1cb35614cd8d6fa52f0e90a894c1af5 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 9 Feb 2021 20:20:34 +0100 Subject: efi_loader: fix get_last_capsule() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix get_last_capsule() leads to writes beyond the stack allocated buffer. This was indicated when enabling the stack protector. utf16_utf8_strcpy() only stops copying when reaching '\0'. The current invocation always writes beyond the end of value[]. The output length of utf16_utf8_strcpy() may be longer than the number of UTF-16 tokens. E.g has "CapsuleКиев" has 11 UTF-16 tokens but 15 UTF-8 tokens. Hence, using utf16_utf8_strcpy() without checking the input may lead to further writes beyond value[]. The current invocation of strict_strtoul() reads beyond the end of value[]. A non-hexadecimal value after "Capsule" (e.g. "CapsuleZZZZ") must result in an error. We cat catch this by checking the return value of strict_strtoul(). A value that is too short after "Capsule" (e.g. "Capsule0") must result in an error. We must check the string length of value[]. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_capsule.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index d39d7310804..b57f0302c59 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -42,20 +42,28 @@ static struct efi_file_handle *bootdev_root; static __maybe_unused unsigned int get_last_capsule(void) { u16 value16[11]; /* "CapsuleXXXX": non-null-terminated */ - char value[11], *p; + char value[5]; efi_uintn_t size; unsigned long index = 0xffff; efi_status_t ret; + int i; size = sizeof(value16); ret = efi_get_variable_int(L"CapsuleLast", &efi_guid_capsule_report, NULL, &size, value16, NULL); - if (ret != EFI_SUCCESS || u16_strncmp(value16, L"Capsule", 7)) + if (ret != EFI_SUCCESS || size != 22 || + u16_strncmp(value16, L"Capsule", 7)) goto err; + for (i = 0; i < 4; ++i) { + u16 c = value16[i + 7]; - p = value; - utf16_utf8_strcpy(&p, value16); - strict_strtoul(&value[7], 16, &index); + if (!c || c > 0x7f) + goto err; + value[i] = c; + } + value[4] = 0; + if (strict_strtoul(value, 16, &index)) + index = 0xffff; err: return index; } -- cgit v1.2.3