public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [PATCH 1/1] efi_loader: validate load option
@ 2020-05-31 20:52 Heinrich Schuchardt
  2020-06-03  2:25 ` AKASHI Takahiro
  0 siblings, 1 reply; 2+ messages in thread
From: Heinrich Schuchardt @ 2020-05-31 20:52 UTC (permalink / raw)
  To: u-boot

For passing the optional data of the load option to the loaded imaged
protocol we need its size.

efi_deserialize_load_option() is changed to return the size of the optional
data.

As a by-product we get a partial validation of the load option.
Checking the length of the device path remains to be implemented.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 cmd/efidebug.c               | 21 +++++++++++-----
 include/efi_loader.h         |  3 ++-
 lib/efi_loader/efi_bootmgr.c | 48 +++++++++++++++++++++++++++++-------
 3 files changed, 56 insertions(+), 16 deletions(-)

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index 32430e62f0..58018f700c 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -694,14 +694,19 @@ static int do_efi_boot_rm(struct cmd_tbl *cmdtp, int flag,
  *
  * Decode the value of UEFI load option variable and print information.
  */
-static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t size)
+static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size)
 {
 	struct efi_load_option lo;
 	char *label, *p;
 	size_t label_len16, label_len;
 	u16 *dp_str;
+	efi_status_t ret;

-	efi_deserialize_load_option(&lo, data);
+	ret = efi_deserialize_load_option(&lo, data, size);
+	if (ret != EFI_SUCCESS) {
+		printf("%ls: invalid load option\n", varname16);
+		return;
+	}

 	label_len16 = u16_strlen(lo.label);
 	label_len = utf16_utf8_strnlen(lo.label, label_len16);
@@ -728,8 +733,7 @@ static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t size)

 	printf("  data:\n");
 	print_hex_dump("    ", DUMP_PREFIX_OFFSET, 16, 1,
-		       lo.optional_data, size + (u8 *)data -
-		       (u8 *)lo.optional_data, true);
+		       lo.optional_data, *size, true);
 	free(label);
 }

@@ -759,7 +763,7 @@ static void show_efi_boot_opt(u16 *varname16)
 						&efi_global_variable_guid,
 						NULL, &size, data));
 		if (ret == EFI_SUCCESS)
-			show_efi_boot_opt_data(varname16, data, size);
+			show_efi_boot_opt_data(varname16, data, &size);
 		free(data);
 	}
 }
@@ -920,7 +924,12 @@ static int show_efi_boot_order(void)
 			goto out;
 		}

-		efi_deserialize_load_option(&lo, data);
+		ret = efi_deserialize_load_option(&lo, data, &size);
+		if (ret != EFI_SUCCESS) {
+			printf("%ls: invalid load option\n", var_name16);
+			ret = CMD_RET_FAILURE;
+			goto out;
+		}

 		label_len16 = u16_strlen(lo.label);
 		label_len = utf16_utf8_strnlen(lo.label, label_len16);
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 9533df26dc..c2cae814b6 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -708,7 +708,8 @@ struct efi_load_option {
 	const u8 *optional_data;
 };

-void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data);
+efi_status_t efi_deserialize_load_option(struct efi_load_option *lo, u8 *data,
+					 efi_uintn_t *size);
 unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data);
 efi_status_t efi_bootmgr_load(efi_handle_t *handle);

diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index fa65445c12..e268e9c4b8 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -38,24 +38,50 @@ static const struct efi_runtime_services *rs;
  *
  * @lo:		pointer to target
  * @data:	serialized data
+ * @size:	size of the load option, on return size of the optional data
+ * Return:	status code
  */
-void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data)
+efi_status_t efi_deserialize_load_option(struct efi_load_option *lo, u8 *data,
+					 efi_uintn_t *size)
 {
+	efi_uintn_t len;
+
+	len = sizeof(u32);
+	if (*size < len + 2 * sizeof(u16))
+		return EFI_INVALID_PARAMETER;
 	lo->attributes = get_unaligned_le32(data);
-	data += sizeof(u32);
+	data += len;
+	*size -= len;

+	len = sizeof(u16);
 	lo->file_path_length = get_unaligned_le16(data);
-	data += sizeof(u16);
+	data += len;
+	*size -= len;

-	/* FIXME */
 	lo->label = (u16 *)data;
-	data += (u16_strlen(lo->label) + 1) * sizeof(u16);
-
-	/* FIXME */
+	len = u16_strnlen(lo->label, *size / sizeof(u16) - 1);
+	if (lo->label[len])
+		return EFI_INVALID_PARAMETER;
+	len = (len + 1) * sizeof(u16);
+	if (*size < len)
+		return EFI_INVALID_PARAMETER;
+	data += len;
+	*size -= len;
+
+	len = lo->file_path_length;
+	if (*size < len)
+		return EFI_INVALID_PARAMETER;
 	lo->file_path = (struct efi_device_path *)data;
-	data += lo->file_path_length;
+	 /*
+	  * TODO: validate device path. There should be an end node within
+	  * the indicated file_path_length.
+	  */
+	data += len;
+	*size -= len;

 	lo->optional_data = data;
+
+	return EFI_SUCCESS;
 }

 /**
@@ -170,7 +196,11 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle)
 	if (!load_option)
 		return EFI_LOAD_ERROR;

-	efi_deserialize_load_option(&lo, load_option);
+	ret = efi_deserialize_load_option(&lo, load_option, &size);
+	if (ret != EFI_SUCCESS) {
+		log_warning("Invalid load option for %ls\n", varname);
+		goto error;
+	}

 	if (lo.attributes & LOAD_OPTION_ACTIVE) {
 		u32 attributes;
--
2.26.2

^ permalink raw reply related	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-06-03  2:25 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-31 20:52 [PATCH 1/1] efi_loader: validate load option Heinrich Schuchardt
2020-06-03  2:25 ` AKASHI Takahiro

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox