[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