[PATCH 07/10] efi_loader: check certificate lengths in efi_image_authenticate()

[Ana Bobillo <anabobillo3@gmail.com>]

From: Ana Bobillo <anabobillo3@gmail.com>

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 <anabobillo3@gmail.com>
---
 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));