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 against
the table:
- a certificate no longer than its header is skipped, but 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.
Turn the check that skipped a certificate no longer than its header
into one that rejects the image when a certificate is no longer than
its header or longer than the rest of the table. EDK2's
DxeImageVerificationLib also rejects 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]>
---
Changes in v3:
- fold the new check and the old "dwLength too small" one into a
single check (Ilias)
lib/efi_loader/efi_image_loader.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/lib/efi_loader/efi_image_loader.c
b/lib/efi_loader/efi_image_loader.c
index 417282698fa..8f48d8fb5a5 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -808,14 +808,26 @@ 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;
- if (wincert->dwLength <= sizeof(*wincert)) {
- log_debug("dwLength too small: %u < %zu\n",
- wincert->dwLength, sizeof(*wincert));
- continue;
+ /*
+ * dwLength includes the WIN_CERTIFICATE header. A certificate
+ * must hold a signature after its header and end within the
+ * certificate table. Reject an image with a corrupted
+ * certificate table: a dwLength of 0 would not advance the
+ * walk, and one past the end of the table would make the
+ * signature run past it.
+ */
+ if (wincert->dwLength <= sizeof(*wincert) ||
+ wincert->dwLength > remaining) {
+ log_err("Invalid certificate length: %u\n",
+ wincert->dwLength);
+ ret = false;
+ goto out;
}
log_debug("WIN_CERTIFICATE_TYPE: 0x%x\n",
