| From 96dccc255b16e9465dbee50b3cef6b3db74d11c8 Mon Sep 17 00:00:00 2001 |
| From: Peter Jones <pjones@redhat.com> |
| Date: Thu, 27 Jul 2023 15:21:31 -0400 |
| Subject: [PATCH] CVE-2023-40548 Fix integer overflow on SBAT section size on |
| 32-bit system |
| |
| In verify_sbat_section(), we do some math on data that comes from the |
| binary being verified - in this case, we add 1 to the size of the |
| ".sbat" section as reported in the section header, which is then used as |
| the input to the size of an allocation. The original value is then used |
| for a size in a memcpy(), which means there's an out-of-bounds write in |
| the overflow case. |
| |
| Due to the type of the variable being size_t, but the type in the |
| section header being uint32_t, this is only plausibly accomplished on |
| 32-bit systems. |
| |
| This patch makes the arithmetic use a checked add operation to avoid |
| overflow. Additionally, it adds a check in verify_buffer_sbat() to |
| guarantee that the data is within the binary. |
| |
| It's not currently known if this is actually exploitable on such |
| systems; the memory layout on a particular machine may further mitigate |
| this scenario. |
| |
| Resolves: CVE-2023-40548 |
| Reported-by: gkirkpatrick@google.com |
| Signed-off-by: Peter Jones <pjones@redhat.com> |
| --- |
| pe.c | 6 +++++- |
| shim.c | 6 ++++++ |
| 2 files changed, 11 insertions(+), 1 deletion(-) |
| |
| diff --git a/pe.c b/pe.c |
| index e15b89f6..b3a9d46f 100644 |
| --- a/pe.c |
| +++ b/pe.c |
| @@ -355,7 +355,11 @@ verify_sbat_section(char *SBATBase, size_t SBATSize) |
| return in_protocol ? EFI_SUCCESS : EFI_SECURITY_VIOLATION; |
| } |
| |
| - sbat_size = SBATSize + 1; |
| + if (checked_add(SBATSize, 1, &sbat_size)) { |
| + dprint(L"SBATSize + 1 would overflow\n"); |
| + return EFI_SECURITY_VIOLATION; |
| + } |
| + |
| sbat_data = AllocatePool(sbat_size); |
| if (!sbat_data) { |
| console_print(L"Failed to allocate .sbat section buffer\n"); |
| diff --git a/shim.c b/shim.c |
| index 3fd1e2a0..84a98cab 100644 |
| --- a/shim.c |
| +++ b/shim.c |
| @@ -743,11 +743,17 @@ verify_buffer_sbat (char *data, int datasize, |
| * and ignore the section if it isn't. */ |
| if (Section->SizeOfRawData && |
| Section->SizeOfRawData >= Section->Misc.VirtualSize) { |
| + uint64_t boundary; |
| SBATBase = ImageAddress(data, datasize, |
| Section->PointerToRawData); |
| SBATSize = Section->SizeOfRawData; |
| dprint(L"sbat section base:0x%lx size:0x%lx\n", |
| SBATBase, SBATSize); |
| + if (checked_add((uint64_t)SBATBase, SBATSize, &boundary) || |
| + (boundary > (uint64_t)data + datasize)) { |
| + perror(L"Section exceeds bounds of image\n"); |
| + return EFI_UNSUPPORTED; |
| + } |
| } |
| } |
| |