secvar: return error if verify_signature runs out of ESLs

Currently, in `verify_signature`, the return code `rc` is initialized
as 0 (our success value). While looping through the ESL's in the given
secvar, the function will break if the remaining data in the secvar is
not enough to contain another ESL. This break from the loop was not
setting a return code, this means that the successful return code
can pass to the end of the function if the first iteration meets
this condition. In other words, if a current secvar has a size that
is less than minimum size for an ESL, than it will approve any update.

In response to this bug, this commit will return an error code if
the described condition is met. Additionally, a test case has been
added to ensure that this unlikely event is handled correctly.

Signed-off-by: Nick Child <nick.child@ibm.com>
Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
Tested-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>

2 files changed, 29 insertions, 1 deletions

diff --git a/libstb/secvar/backend/edk2-compat-process.c b/libstb/secvar/backend/edk2-compat-process.c
index 3fab634..a8d1635 100644
--- a/libstb/secvar/backend/edk2-compat-process.c
+++ b/libstb/secvar/backend/edk2-compat-process.c
@@ -512,8 +512,11 @@ static int verify_signature(const struct efi_variable_authentication_2 *auth,
 	/* Variable is not empty */
 	while (eslvarsize > 0) {
 		prlog(PR_DEBUG, "esl var size is %d offset is %d\n", eslvarsize, offset);
-		if (eslvarsize < sizeof(EFI_SIGNATURE_LIST))
+		if (eslvarsize < sizeof(EFI_SIGNATURE_LIST)) {
+			rc = OPAL_INTERNAL_ERROR;
+			prlog(PR_ERR, "ESL data is corrupted\n");
 			break;
+		}
 
 		/* Calculate the size of the ESL */
 		eslsize = get_esl_signature_list_size(avar->data + offset,
diff --git a/libstb/secvar/test/secvar-test-edk2-compat.c b/libstb/secvar/test/secvar-test-edk2-compat.c
index 035e20a..65ed549 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;
+	size_t tmp_size;
 	char empty[64] = {0};
 
 	/* The sequence of test cases here is important to ensure that
@@ -214,6 +215,30 @@ int run_test()
 	tmp = find_secvar("db", 3, &variable_bank);
 	ASSERT(NULL != tmp);
 
+	/* Add db, should fail with no KEK and invalid PK size */
+	printf("Add db, corrupt PK");
+	/* Somehow PK gets assigned wrong size */
+	tmp = find_secvar("PK", 3, &variable_bank);
+	ASSERT(NULL != tmp);
+	tmp_size = tmp->data_size;
+	tmp->data_size = sizeof(EFI_SIGNATURE_LIST) - 1;
+	tmp = new_secvar("db", 3, DB_auth, DB_auth_len, 0);
+	ASSERT(0 == edk2_compat_validate(tmp));
+	list_add_tail(&update_bank, &tmp->link);
+	ASSERT(1 == list_length(&update_bank));
+
+	rc = edk2_compat_process(&variable_bank, &update_bank);
+	ASSERT(OPAL_INTERNAL_ERROR == rc);
+	ASSERT(5 == list_length(&variable_bank));
+	ASSERT(0 == list_length(&update_bank));
+	tmp = find_secvar("db", 3, &variable_bank);
+	ASSERT(NULL != tmp);
+	ASSERT(0 == tmp->data_size);
+	/* Restore PK data size */
+	tmp = find_secvar("PK", 3, &variable_bank);
+	ASSERT(NULL != tmp);
+	tmp->data_size = tmp_size;
+
 	/* Add trimmed KEK, .process(), should fail. */
 	printf("Add trimmed KEK\n");
 	tmp = new_secvar("KEK", 4, trimmedKEK_auth, trimmedKEK_auth_len, 0);