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 e15b89f..b3a9d46 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 3fd1e2a..84a98ca 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;
+			}
 		}
 	}
 
-- 
2.33.0