Introduce validation of bgenv prior to its usage

The parsing of user variables assumes sane input so far and can be
mislead to out-of-bounds accesses, including writes. Address this by
always validating a bgenv after reading it from a partition or a file.
If an invalid bgenv is found, it is cleared to zero internally so that
the existing code will always operate against a sane state.

Include the CRC32 validation in the new helper as well which also
ensures that the checksum is tested when operating against a specific
file.

Reported by Code Intelligence.

Addresses CVE-2023-39950

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

env/env_api_fat.c

@@ -51,6 +51,33 @@ void bgenv_be_verbose(bool v)
 	ebgpart_beverbose(v);
 }
 
+static void clear_envdata(BG_ENVDATA *data)
+{
+	memset(data, 0, sizeof(BG_ENVDATA));
+	data->crc32 = crc32(0, (Bytef *)data,
+			    sizeof(BG_ENVDATA) - sizeof(data->crc32));
+}
+
+bool validate_envdata(BG_ENVDATA *data)
+{
+	uint32_t sum = crc32(0, (Bytef *)data,
+			     sizeof(BG_ENVDATA) - sizeof(data->crc32));
+
+	if (data->crc32 != sum) {
+		VERBOSE(stderr, "Invalid CRC32!\n");
+		/* clear invalid environment */
+		clear_envdata(data);
+		return false;
+	}
+	if (!bgenv_validate_uservars(data->userdata)) {
+		VERBOSE(stderr, "Corrupt uservars!\n");
+		/* clear invalid environment */
+		clear_envdata(data);
+		return false;
+	}
+	return true;
+}
+
 bool read_env(CONFIG_PART *part, BG_ENVDATA *env)
 {
 	if (!part) {
@@ -86,10 +113,16 @@ bool read_env(CONFIG_PART *part, BG_ENVDATA *env)
 	if (part->not_mounted) {
 		unmount_partition(part);
 	}
+	if (result == false) {
+		clear_envdata(env);
+		return false;
+	}
+
 	/* enforce NULL-termination of strings */
 	env->kernelfile[ENV_STRING_LENGTH - 1] = 0;
 	env->kernelparams[ENV_STRING_LENGTH - 1] = 0;
-	return result;
+
+	return validate_envdata(env);
 }
 
 bool write_env(CONFIG_PART *part, BG_ENVDATA *env)
@@ -147,15 +180,6 @@ bool bgenv_init(void)
 	}
 	for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
 		read_env(&config_parts[i], &envdata[i]);
-		uint32_t sum = crc32(0, (Bytef *)&envdata[i],
-		    sizeof(BG_ENVDATA) - sizeof(envdata[i].crc32));
-		if (envdata[i].crc32 != sum) {
-			VERBOSE(stderr, "Invalid CRC32!\n");
-			/* clear invalid environment */
-			memset(&envdata[i], 0, sizeof(BG_ENVDATA));
-			envdata[i].crc32 = crc32(0, (Bytef *)&envdata[i],
-			    sizeof(BG_ENVDATA) - sizeof(envdata[i].crc32));
-		}
 	}
 	initialized = true;
 	return true;

env/uservars.c

@@ -72,6 +72,35 @@ void bgenv_map_uservar(uint8_t *udata, char **key, uint64_t *type, uint8_t **val
 	}
 }
 
+bool bgenv_validate_uservars(uint8_t *udata)
+{
+	uint32_t spaceleft = ENV_MEM_USERVARS;
+
+	while (*udata) {
+		uint32_t key_len = strnlen((char *)udata, spaceleft);
+
+		/* we need space for the key string + null termination +
+		 * the payload size field */
+		if (key_len + 1 + sizeof(uint32_t) >= spaceleft) {
+			return false;
+		}
+
+		spaceleft -= key_len + 1;
+		udata += key_len + 1;
+
+		uint32_t payload_size = *(uint32_t *)udata;
+
+		/* the payload must leave at least one byte free */
+		if (payload_size >= spaceleft) {
+			return false;
+		}
+
+		spaceleft -= payload_size;
+		udata += payload_size;
+	}
+	return true;
+}
+
 void bgenv_serialize_uservar(uint8_t *p, char *key, uint64_t type, void *data,
 			    uint32_t record_size)
 {

include/env_api.h

@@ -95,3 +95,5 @@ extern int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
 extern int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
 		     uint32_t datalen);
 extern uint8_t *bgenv_find_uservar(uint8_t *userdata, char *key);
+
+extern bool validate_envdata(BG_ENVDATA *data);

include/uservars.h

@@ -14,6 +14,7 @@
 
 #pragma once
 
+#include <stdbool.h>
 #include <stdint.h>
 
 void bgenv_map_uservar(uint8_t *udata, char **key, uint64_t *type,
@@ -35,3 +36,5 @@ uint8_t *bgenv_uservar_realloc(uint8_t *udata, uint32_t new_rsize,
 			       uint8_t *p);
 void bgenv_del_uservar(uint8_t *udata, uint8_t *var);
 uint32_t bgenv_user_free(uint8_t *udata);
+
+bool bgenv_validate_uservars(uint8_t *udata);

tools/bg_envtools.c

@@ -155,9 +155,13 @@ bool get_env(char *configfilepath, BG_ENVDATA *data)
 			"Error closing environment file after reading.\n");
 	};
 
+	if (result == false) {
+		return false;
+	}
+
 	/* enforce NULL-termination of strings */
 	data->kernelfile[ENV_STRING_LENGTH - 1] = 0;
 	data->kernelparams[ENV_STRING_LENGTH - 1] = 0;
 
-	return result;
+	return validate_envdata(data);
 }