Merge branch 'assorted-fit-validation-fixes' into next

This brings in a number of FIT image validation fixes from Anton Ivanov
<[email protected]> that have security implications.

3 files changed, 92 insertions, 8 deletions

diff --git a/boot/fdt_region.c b/boot/fdt_region.c
index 295ea08ac91..dd6e87925be 100644
--- a/boot/fdt_region.c
+++ b/boot/fdt_region.c
@@ -69,6 +69,8 @@ int fdt_find_regions(const void *fdt, char * const inc[], int inc_count,
 			include = want >= 2;
 			stop_at = offset;
 			prop = fdt_get_property_by_offset(fdt, offset, NULL);
+			if (!prop)
+				return -FDT_ERR_BADSTRUCTURE;
 			str = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
 			if (!str)
 				return -FDT_ERR_BADSTRUCTURE;
@@ -86,6 +88,8 @@ int fdt_find_regions(const void *fdt, char * const inc[], int inc_count,
 			if (depth == FDT_MAX_DEPTH)
 				return -FDT_ERR_BADSTRUCTURE;
 			name = fdt_get_name(fdt, offset, &len);
+			if (!name)
+				return len;
 
 			/* The root node must have an empty name */
 			if (!depth && *name)
@@ -271,7 +275,11 @@ int fdt_add_alias_regions(const void *fdt, struct fdt_region *region, int count,
 		int target, next;
 
 		prop = fdt_get_property_by_offset(fdt, offset, NULL);
+		if (!prop)
+			return -FDT_ERR_BADSTRUCTURE;
 		name = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
+		if (!name)
+			return -FDT_ERR_BADSTRUCTURE;
 		target = fdt_path_offset(fdt, name);
 		if (!region_list_contains_offset(info, fdt, target))
 			continue;
@@ -520,7 +528,11 @@ int fdt_next_region(const void *fdt,
 		case FDT_PROP:
 			stop_at = offset;
 			prop = fdt_get_property_by_offset(fdt, offset, NULL);
+			if (!prop)
+				return -FDT_ERR_BADSTRUCTURE;
 			str = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
+			if (!str)
+				return -FDT_ERR_BADSTRUCTURE;
 			val = h_include(priv, fdt, last_node, FDT_IS_PROP, str,
 					    strlen(str) + 1);
 			if (val == -1) {
@@ -553,6 +565,9 @@ int fdt_next_region(const void *fdt,
 			if (p.depth == FDT_MAX_DEPTH)
 				return -FDT_ERR_BADSTRUCTURE;
 			name = fdt_get_name(fdt, offset, &len);
+			if (!name)
+				return len;
+
 			if (p.end - path + 2 + len >= path_len)
 				return -FDT_ERR_NOSPACE;
 
diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c
index 433df20281f..9b5ab754561 100644
--- a/boot/image-fit-sig.c
+++ b/boot/image-fit-sig.c
@@ -452,6 +452,8 @@ static int fit_config_check_sig(const void *fit, int noffset, int conf_noffset,
 	int max_regions;
 	char path[200];
 	int count;
+	int len;
+	uint32_t size;
 
 	debug("%s: fdt=%p, conf='%s', sig='%s'\n", __func__, key_blob,
 	      fit_get_name(fit, noffset, NULL),
@@ -506,14 +508,27 @@ static int fit_config_check_sig(const void *fit, int noffset, int conf_noffset,
 	}
 
 	/* Add the strings */
-	strings = fdt_getprop(fit, noffset, "hashed-strings", NULL);
+	strings = fdt_getprop(fit, noffset, "hashed-strings", &len);
 	if (strings) {
+		if (len < (int)(2 * sizeof(fdt32_t))) {
+			*err_msgp = "Invalid hashed-strings property";
+			return -1;
+		}
+		size = fdt32_to_cpu(strings[1]);
+		/*
+		 * The offset should be already validated by fdt_check_header();
+		 * validate the size here.
+		 */
+		if (size > fdt_size_dt_strings(fit)) {
+			*err_msgp = "Strings region is out of bounds";
+			return -1;
+		}
 		/*
 		 * The strings region offset must be a static 0x0.
 		 * This is set in tool/image-host.c
 		 */
 		fdt_regions[count].offset = fdt_off_dt_strings(fit);
-		fdt_regions[count].size = fdt32_to_cpu(strings[1]);
+		fdt_regions[count].size = size;
 		count++;
 	}
 
diff --git a/boot/image-fit.c b/boot/image-fit.c
index 773eb1857c5..937654b6223 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -44,6 +44,7 @@ DECLARE_GLOBAL_DATA_PTR;
 #include <bootm.h>
 #include <image.h>
 #include <bootstage.h>
+#include <fdt_region.h>
 #include <upl.h>
 #include <u-boot/crc.h>
 
@@ -1072,23 +1073,72 @@ int fit_image_get_data(const void *fit, int noffset, const void **data,
 	int offset;
 	int len;
 	int ret;
+	size_t fdt_total_size_aligned;
+	uintptr_t max_offset;
 
 	if (!fit_image_get_data_position(fit, noffset, &offset)) {
+		if (offset < 0) {
+			printf("Invalid external data position: %d\n", offset);
+			return -EINVAL;
+		}
+
 		external_data = true;
 	} else if (!fit_image_get_data_offset(fit, noffset, &offset)) {
-		external_data = true;
 		/*
 		 * For FIT with external data, figure out where
 		 * the external images start. This is the base
 		 * for the data-offset properties in each image.
 		 */
-		offset += ((fdt_totalsize(fit) + 3) & ~3);
+		fdt_total_size_aligned = ((fdt_totalsize(fit) + 3) & ~3);
+		/* The resulting offset cannot exceed INT_MAX */
+		if (offset < 0 || fdt_total_size_aligned > INT_MAX - offset) {
+			printf("Invalid external data offset: %d\n", offset);
+			return -EINVAL;
+		}
+		offset += fdt_total_size_aligned;
+
+		external_data = true;
 	}
 
 	if (external_data) {
 		debug("External Data\n");
+
+		max_offset = UINTPTR_MAX - (uintptr_t)fit;
+		/* Check that external data offset is within the addressable range */
+		if (offset > max_offset) {
+			printf("Invalid external data offset: %d\n", offset);
+			return -EINVAL;
+		}
+
 		ret = fit_image_get_data_size(fit, noffset, &len);
 		if (!ret) {
+			if (len < 0) {
+				printf("Invalid external data size: %d\n", len);
+				return -EINVAL;
+			}
+			/*
+			 * For non-signed FIT images, we can only check that
+			 * (offset + len) doesn't exceed the addressable range.
+			 * For signed FITs, we can additionally check that
+			 * (offset + len) doesn't exceed the allowed FIT image
+			 * maximum size.
+			 */
+			if (len > max_offset - offset
+			/*
+			 * #if (not a runtime if) is required: FIT_SIGNATURE_MAX_SIZE
+			 * depends on FIT_SIGNATURE, so CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE)
+			 * is undefined when signing is disabled and referencing it
+			 * here would fail to compile.
+			 */
+#if CONFIG_IS_ENABLED(FIT_SIGNATURE)
+			    || offset > CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE) ||
+			    len > CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE) - offset
+#endif
+			) {
+				printf("FIT external data is out of bounds (offset=%d, size=%d)\n",
+				       offset, len);
+				return -EINVAL;
+			}
 			*data = fit + offset;
 			*size = len;
 		}
@@ -1637,20 +1687,24 @@ int fit_image_check_comp(const void *fit, int noffset, uint8_t comp)
  *
  * @fit: FIT to check
  * @parent: Parent node to check
- * Return: 0 if OK, -EADDRNOTAVAIL is a node has a name containing '@'
+ * @depth: Current recursion depth
+ * Return: 0 if OK, or error value
  */
-static int fdt_check_no_at(const void *fit, int parent)
+static int fdt_check_no_at(const void *fit, int parent, int depth)
 {
 	const char *name;
 	int node;
 	int ret;
 
+	if (depth >= FDT_MAX_DEPTH)
+		return -FDT_ERR_BADSTRUCTURE;
+
 	name = fdt_get_name(fit, parent, NULL);
 	if (!name || strchr(name, '@'))
 		return -EADDRNOTAVAIL;
 
 	fdt_for_each_subnode(node, fit, parent) {
-		ret = fdt_check_no_at(fit, node);
+		ret = fdt_check_no_at(fit, node, depth + 1);
 		if (ret)
 			return ret;
 	}
@@ -1707,7 +1761,7 @@ int fit_check_format(const void *fit, ulong size)
 		 * attached. Protect against this by disallowing unit addresses.
 		 */
 		if (!ret && CONFIG_IS_ENABLED(FIT_SIGNATURE)) {
-			ret = fdt_check_no_at(fit, 0);
+			ret = fdt_check_no_at(fit, 0, 0);
 
 			if (ret) {
 				log_debug("FIT check error %d\n", ret);