[PATCH 1/1] efi_loader: check for image truncation

[PATCH 1/1] efi_loader: check for image truncation

[Heinrich Schuchardt]

Check in efi_load_pe() that the image size parameter is at least as large
as the image size indicated in the optional PE header.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_image_loader.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
index 94f76ef6b8..e55ade3291 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -761,6 +761,29 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
 		goto err;
 	}

+	if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
+		IMAGE_NT_HEADERS64 *nt64 = (void *)nt;
+
+		if ((size_t)opt->SizeOfImage > efi_size) {
+			log_err("Truncated Image\n");
+			ret = EFI_LOAD_ERROR;
+			goto err;
+		}
+	} else if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+		IMAGE_OPTIONAL_HEADER32 *opt = &nt->OptionalHeader;
+
+		if ((size_t)opt->SizeOfImage > efi_size) {
+			log_err("Truncated Image\n");
+			ret = EFI_LOAD_ERROR;
+			goto err;
+		}
+	} else {
+		log_err("Invalid optional header magic %x\n",
+			nt->OptionalHeader.Magic);
+		ret = EFI_LOAD_ERROR;
+		goto err;
+	}
+
 	/* Authenticate an image */
 	if (efi_image_authenticate(efi, efi_size)) {
 		handle->auth_status = EFI_IMAGE_AUTH_PASSED;
@@ -810,11 +833,6 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
 		rel_size = opt->DataDirectory[rel_idx].Size;
 		rel = efi_reloc + opt->DataDirectory[rel_idx].VirtualAddress;
 		virt_size = ALIGN(virt_size, opt->SectionAlignment);
-	} else {
-		log_err("Invalid optional header magic %x\n",
-			nt->OptionalHeader.Magic);
-		ret = EFI_LOAD_ERROR;
-		goto err;
 	}

 	/* Copy PE headers */
--
2.28.0

[PATCH 1/1] efi_loader: check for image truncation

[AKASHI Takahiro]

On Thu, Aug 27, 2020 at 06:24:35PM +0200, Heinrich Schuchardt wrote:
> Check in efi_load_pe() that the image size parameter is at least as large
> as the image size indicated in the optional PE header.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_image_loader.c | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
> index 94f76ef6b8..e55ade3291 100644
> --- a/lib/efi_loader/efi_image_loader.c
> +++ b/lib/efi_loader/efi_image_loader.c
> @@ -761,6 +761,29 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
>  		goto err;
>  	}
> 
> +	if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
> +		IMAGE_NT_HEADERS64 *nt64 = (void *)nt;
> +
> +		if ((size_t)opt->SizeOfImage > efi_size) {
> +			log_err("Truncated Image\n");
> +			ret = EFI_LOAD_ERROR;
> +			goto err;
> +		}
> +	} else if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> +		IMAGE_OPTIONAL_HEADER32 *opt = &nt->OptionalHeader;
> +
> +		if ((size_t)opt->SizeOfImage > efi_size) {
> +			log_err("Truncated Image\n");
> +			ret = EFI_LOAD_ERROR;
> +			goto err;
> +		}
> +	} else {
> +		log_err("Invalid optional header magic %x\n",
> +			nt->OptionalHeader.Magic);
> +		ret = EFI_LOAD_ERROR;
> +		goto err;
> +	}

Please remove the same message, which is now redundant,
in efi_image_parse().

-Takahiro Akashi

> +
>  	/* Authenticate an image */
>  	if (efi_image_authenticate(efi, efi_size)) {
>  		handle->auth_status = EFI_IMAGE_AUTH_PASSED;
> @@ -810,11 +833,6 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
>  		rel_size = opt->DataDirectory[rel_idx].Size;
>  		rel = efi_reloc + opt->DataDirectory[rel_idx].VirtualAddress;
>  		virt_size = ALIGN(virt_size, opt->SectionAlignment);
> -	} else {
> -		log_err("Invalid optional header magic %x\n",
> -			nt->OptionalHeader.Magic);
> -		ret = EFI_LOAD_ERROR;
> -		goto err;
>  	}
> 
>  	/* Copy PE headers */
> --
> 2.28.0
>