From: Ana Bobillo <[email protected]>
efi_image_authenticate() walks the certificate table of an image by
dwLength, rounded up to a multiple of 8, without checking it:
- a dwLength of 0, or one that wraps around to 0 when rounded up,
does not advance the walk, so U-Boot hangs on such an image;
- a dwLength larger than the rest of the table makes
pkcs7_parse_message() read past the end of the table and of the
image.
As EDK2's DxeImageVerificationLib does, reject an image whose
certificate table holds a certificate shorter than its header or
longer than the rest of the table.
Fixes: 4540dabdcaca ("efi_loader: image_loader: support image authentication")
Fixes: 1a44b7059c18 ("efi_loader: image_loader: add a check against certificate
type of authenticode")
Signed-off-by: Ana Bobillo <[email protected]>
---
lib/efi_loader/efi_image_loader.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/efi_image_loader.c
b/lib/efi_loader/efi_image_loader.c
index c0677b440e8..c4f64f102e8 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -822,10 +822,27 @@ static bool efi_image_authenticate(void *efi, size_t
efi_size)
for (wincert = wincerts, wincerts_end = (u8 *)wincerts + wincerts_len;
(u8 *)wincert < wincerts_end;
wincert = (WIN_CERTIFICATE *)
- ((u8 *)wincert + ALIGN(wincert->dwLength, 8))) {
- if ((u8 *)wincert + sizeof(*wincert) >= wincerts_end)
+ ((u8 *)wincert + ALIGN((size_t)wincert->dwLength, 8))) {
+ size_t remaining = wincerts_end - (u8 *)wincert;
+
+ if (sizeof(*wincert) >= remaining)
break;
+ /*
+ * dwLength includes the WIN_CERTIFICATE header. A shorter one
+ * would not advance the walk, and one longer than the rest of
+ * the certificate table would make the signature run past its
+ * end. Like EDK2, reject an image with such a corrupted
+ * certificate table.
+ */
+ if (wincert->dwLength < sizeof(*wincert) ||
+ wincert->dwLength > remaining) {
+ log_err("Invalid certificate length: %u\n",
+ wincert->dwLength);
+ ret = false;
+ goto out;
+ }
+
if (wincert->dwLength <= sizeof(*wincert)) {
log_debug("dwLength too small: %u < %zu\n",
wincert->dwLength, sizeof(*wincert));
