[PATCH 22.11 21.11] test/crypto: fix vector global buffer overflow

[PATCH 22.11 21.11] test/crypto: fix vector global buffer overflow

[Ciara Power]

When doing a memcpy of the test vector into a local union variable,
the size of the union was used. This meant extra bytes were being copied
from the test vector address in the case the vector was smaller in size
than the union. This caused a global buffer overflow error detected by
Address Sanitizer.

To fix this, the size of the test vector is also stored alongside the
address, so when copying takes place, the minimum of the union and test
vector can be used as the size reference.

Fixes: 488f5a23c219 ("test/crypto: check asymmetric crypto")

Signed-off-by: Ciara Power <ciara.power@intel.com>
---
This issue was fixed by a rework in 2023, so this fix is only applicable
to 21.11 and 22.11 LTS releases that are currently maintained.
It is not applicable to 23.11 LTS, or current upstream releases.
---
 app/test/test_cryptodev_asym.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/app/test/test_cryptodev_asym.c b/app/test/test_cryptodev_asym.c
index 67659cd1a6..b3345c0a39 100644
--- a/app/test/test_cryptodev_asym.c
+++ b/app/test/test_cryptodev_asym.c
@@ -54,11 +54,15 @@ union test_case_structure {
 	struct rsa_test_data_2 rsa_data;
 };
 
+struct vector_details {
+	uint32_t vector_size;
+	const void *address;
+};
 struct test_cases_array {
 	uint32_t size;
-	const void *address[TEST_VECTOR_SIZE];
+	struct vector_details details[TEST_VECTOR_SIZE];
 };
-static struct test_cases_array test_vector = {0, { NULL } };
+static struct test_cases_array test_vector = {0, {} };
 
 static uint32_t test_index;
 
@@ -513,14 +517,14 @@ test_cryptodev_asym_op(struct crypto_testsuite_params_asym *ts_params,
 }
 
 static int
-test_one_case(const void *test_case, int sessionless)
+test_one_case(struct vector_details test_case, int sessionless)
 {
 	int status = TEST_SUCCESS, i = 0;
 	char test_msg[ASYM_TEST_MSG_LEN + 1];
 
 	/* Map the case to union */
 	union test_case_structure tc;
-	memcpy(&tc, test_case, sizeof(tc));
+	rte_memcpy(&tc, test_case.address, RTE_MIN(sizeof(tc), test_case.vector_size));
 
 	if (tc.modex.xform_type == RTE_CRYPTO_ASYM_XFORM_MODEX
 			|| tc.modex.xform_type == RTE_CRYPTO_ASYM_XFORM_MODINV) {
@@ -572,7 +576,8 @@ load_test_vectors(void)
 				"TEST_VECTOR_SIZE too small\n");
 			return -1;
 		}
-		test_vector.address[test_vector.size] = &modex_test_case[i];
+		test_vector.details[test_vector.size].address = &modex_test_case[i];
+		test_vector.details[test_vector.size].vector_size = sizeof(modex_test_case[i]);
 		test_vector.size++;
 	}
 	/* Load MODINV vector*/
@@ -583,7 +588,8 @@ load_test_vectors(void)
 				"TEST_VECTOR_SIZE too small\n");
 			return -1;
 		}
-		test_vector.address[test_vector.size] = &modinv_test_case[i];
+		test_vector.details[test_vector.size].address = &modinv_test_case[i];
+		test_vector.details[test_vector.size].vector_size = sizeof(modinv_test_case[i]);
 		test_vector.size++;
 	}
 	/* Load RSA vector*/
@@ -594,7 +600,8 @@ load_test_vectors(void)
 				"TEST_VECTOR_SIZE too small\n");
 			return -1;
 		}
-		test_vector.address[test_vector.size] = &rsa_test_case_list[i];
+		test_vector.details[test_vector.size].address = &rsa_test_case_list[i];
+		test_vector.details[test_vector.size].vector_size = sizeof(rsa_test_case_list[i]);
 		test_vector.size++;
 	}
 	return 0;
@@ -619,12 +626,12 @@ test_one_by_one(void)
 	/* Go through all test cases */
 	test_index = 0;
 	for (i = 0; i < test_vector.size; i++) {
-		if (test_one_case(test_vector.address[i], 0) != TEST_SUCCESS)
+		if (test_one_case(test_vector.details[i], 0) != TEST_SUCCESS)
 			status = TEST_FAILED;
 	}
 	if (sessionless) {
 		for (i = 0; i < test_vector.size; i++) {
-			if (test_one_case(test_vector.address[i], 1)
+			if (test_one_case(test_vector.details[i], 1)
 					!= TEST_SUCCESS)
 				status = TEST_FAILED;
 		}
-- 
2.25.1

Re: [PATCH 22.11 21.11] test/crypto: fix vector global buffer overflow

[Kevin Traynor]

On 07/05/2024 14:40, Ciara Power wrote:
> When doing a memcpy of the test vector into a local union variable,
> the size of the union was used. This meant extra bytes were being copied
> from the test vector address in the case the vector was smaller in size
> than the union. This caused a global buffer overflow error detected by
> Address Sanitizer.
> 
> To fix this, the size of the test vector is also stored alongside the
> address, so when copying takes place, the minimum of the union and test
> vector can be used as the size reference.
> 
> Fixes: 488f5a23c219 ("test/crypto: check asymmetric crypto")
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> ---
> This issue was fixed by a rework in 2023, so this fix is only applicable
> to 21.11 and 22.11 LTS releases that are currently maintained.
> It is not applicable to 23.11 LTS, or current upstream releases.
> ---
>  app/test/test_cryptodev_asym.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 

Thanks Ciara, will apply to the tree and it will be part of the next
release.