From afdc5039de0a4a3a40162a32daa070f94a883f09 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Thu, 27 Jul 2023 14:58:55 -0400
Subject: [PATCH] CVE-2023-40549 Authenticode: verify that the signature header
 is in bounds.

In the validation logic in verify_buffer_authenticode(), there is yet
another case where we need to guarantee an object is in the binary but
we're only validating the pointer to it.  In this case, we're validating
that the actual signature data is in the binary, but unfortunately we
failed to validate that the header describing it is, so a malformed
binary can cause us to take an out-of-bounds read (probably but not
necessarily on the same page) past the end of the buffer.

This patch adds a bounds check to verify that the signature is
actually within the bounds.

It seems unlikely this can be used for more than a denial of service,
and if you can get shim to try to verify a malformed binary, you've
effectively already accomplished a DoS.

Resolves: CVE-2023-40549
Reported-by: gkirkpatrick@google.com
Signed-off-by: Peter Jones <pjones@redhat.com>
---
 shim.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/shim.c b/shim.c
index 3a97067..3fd1e2a 100644
--- a/shim.c
+++ b/shim.c
@@ -627,11 +627,13 @@ verify_buffer_authenticode (char *data, int datasize,
 		return EFI_SECURITY_VIOLATION;
 	}
 
-	if (context->SecDir->Size >= size) {
+	if (checked_add(context->SecDir->Size, context->SecDir->VirtualAddress, &offset) ||
+	    offset > size) {
 		perror(L"Certificate Database size is too large\n");
 		return EFI_INVALID_PARAMETER;
 	}
 
+	offset = 0;
 	ret_efi_status = EFI_NOT_FOUND;
 	do {
 		WIN_CERTIFICATE_EFI_PKCS *sig = NULL;
@@ -642,6 +644,11 @@ verify_buffer_authenticode (char *data, int datasize,
 		if (!sig)
 			break;
 
+		if ((uint64_t)&sig[1] > (uint64_t)data + datasize) {
+			perror(L"Certificate size is too large for secruity database");
+			return EFI_INVALID_PARAMETER;
+		}
+
 		sz = offset + offsetof(WIN_CERTIFICATE_EFI_PKCS, Hdr.dwLength)
 		     + sizeof(sig->Hdr.dwLength);
 		if (sz > context->SecDir->Size) {
-- 
2.33.0