| 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 3a97067b..3fd1e2a0 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) { |