[Skiboot] [PATCH v2 1/7] secvar/backend: Don't overread short variables in validate

Daniel Axtens dja at axtens.net
Thu Jul 8 17:10:34 AEST 2021


Fix an OOB read caught by our fuzzer.

It might be good future work to change function signatures to pass
some size data around explictly?

Signed-off-by: Daniel Axtens <dja at axtens.net>
---
 libstb/secvar/backend/edk2-compat.c          |  3 +++
 libstb/secvar/test/secvar-test-edk2-compat.c | 10 ++++++++++
 2 files changed, 13 insertions(+)

diff --git a/libstb/secvar/backend/edk2-compat.c b/libstb/secvar/backend/edk2-compat.c
index 9e61fbc60ff9..bfa2659e526b 100644
--- a/libstb/secvar/backend/edk2-compat.c
+++ b/libstb/secvar/backend/edk2-compat.c
@@ -280,6 +280,9 @@ static int edk2_compat_validate(struct secvar *var)
 			&& !key_equals(var->key, "dbx"))
 		return OPAL_PARAMETER;
 
+	if (var->data_size < sizeof(struct efi_variable_authentication_2))
+		return OPAL_PARAMETER;
+
 	/* Check that signature type is PKCS7 */
 	if (!is_pkcs7_sig_format(var->data))
 		return OPAL_PARAMETER;
diff --git a/libstb/secvar/test/secvar-test-edk2-compat.c b/libstb/secvar/test/secvar-test-edk2-compat.c
index 3532eb6247d3..7ea2df40b9c2 100644
--- a/libstb/secvar/test/secvar-test-edk2-compat.c
+++ b/libstb/secvar/test/secvar-test-edk2-compat.c
@@ -90,6 +90,7 @@ int run_test()
 {
 	int rc = -1;
 	struct secvar *tmp;
+	void *data;
 	size_t tmp_size;
 	char empty[64] = {0};
 
@@ -254,6 +255,15 @@ int run_test()
 	ASSERT(NULL != tmp);
 	ASSERT(0 == tmp->data_size);
 
+	printf("Try truncated KEK < size of variable header:\n");
+	data = malloc(32);
+	memcpy(data, KEK_auth, 32);
+	tmp = new_secvar("KEK", 4, data, 32, 0);
+	rc = edk2_compat_validate(tmp);
+	ASSERT(OPAL_PARAMETER == rc);
+	free (data);
+	dealloc_secvar (tmp);
+
 	/* Add valid KEK, .process(), succeeds. */
 	printf("Add KEK");
 	tmp = new_secvar("KEK", 4, KEK_auth, KEK_auth_len, 0);
-- 
2.30.2



More information about the Skiboot mailing list