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>

2 files changed, 11 insertions, 1 deletions

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;
+			}
 		}
 	}