pe: Perform image verification earlier when loading grub

The second stage loader was being verified after loading it into
memory. As an additional hardening measure to avoid performing risky
memcpys using header fields from a potentially specially crafted image,
perform the verification before this so that it can be rejected earlier.

Signed-off-by: Chris Coulson <chris.coulson@canonical.com>

1 files changed, 25 insertions, 17 deletions

diff --git a/pe.c b/pe.c
index 1eb3f59a..1d120f2d 100644
--- a/pe.c
+++ b/pe.c
@@ -1106,7 +1106,31 @@ handle_image (void *data, unsigned int datasize,
 	}
 
 	/*
-	 * We only need to verify the binary if we're in secure mode
+	 * Perform the image verification before we start copying data around
+	 * in order to load it.
+	 */
+	if (secure_mode ()) {
+		efi_status = verify_buffer(data, datasize, &context, sha256hash,
+					   sha1hash);
+
+		if (EFI_ERROR(efi_status)) {
+			if (verbose)
+				console_print(L"Verification failed: %r\n", efi_status);
+			else
+				console_error(L"Verification failed", efi_status);
+			return efi_status;
+		} else {
+			if (verbose)
+				console_print(L"Verification succeeded\n");
+		}
+	}
+
+	/*
+	 * Calculate the hash for the TPM measurement.
+	 * XXX: We're computing these twice in secure boot mode when the
+	 *  buffers already contain the previously computed hashes. Also,
+	 *  this is only useful for the TPM1.2 case. We should try to fix
+	 *  this in a follow-up.
 	 */
 	efi_status = generate_hash(data, datasize, &context, sha256hash,
 				   sha1hash);
@@ -1287,22 +1311,6 @@ handle_image (void *data, unsigned int datasize,
 		}
 	}
 
-	if (secure_mode ()) {
-		efi_status = verify_buffer(data, datasize, &context, sha256hash,
-					   sha1hash);
-
-		if (EFI_ERROR(efi_status)) {
-			if (verbose)
-				console_print(L"Verification failed: %r\n", efi_status);
-			else
-				console_error(L"Verification failed", efi_status);
-			return efi_status;
-		} else {
-			if (verbose)
-				console_print(L"Verification succeeded\n");
-		}
-	}
-
 	if (context.NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC) {
 		perror(L"Image has no relocation entry\n");
 		FreePool(buffer);