[Skiboot] [PATCH v2 4/4] secvar: return error if verify_signature runs out of ESLs

[Nick Child]

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 at ibm.com>
---
 libstb/secvar/backend/edk2-compat-process.c  |  5 +++-
 libstb/secvar/test/secvar-test-edk2-compat.c | 25 ++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/libstb/secvar/backend/edk2-compat-process.c b/libstb/secvar/backend/edk2-compat-process.c
index ff445d45..056367ee 100644
--- a/libstb/secvar/backend/edk2-compat-process.c
+++ b/libstb/secvar/backend/edk2-compat-process.c
@@ -481,8 +481,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 size is %zu offset is %zu\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 */
 		rc = get_esl_signature_list(avar->data + offset, eslvarsize, NULL, &eslsize);
diff --git a/libstb/secvar/test/secvar-test-edk2-compat.c b/libstb/secvar/test/secvar-test-edk2-compat.c
index d401f952..f1420200 100644
--- a/libstb/secvar/test/secvar-test-edk2-compat.c
+++ b/libstb/secvar/test/secvar-test-edk2-compat.c
@@ -89,6 +89,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
@@ -213,6 +214,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);
-- 
2.25.1