From 02cb59f74a04f217acb613d54991492a46b27a91 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Thu, 1 Aug 2024 15:10:39 -0400 Subject: [PATCH] Fix issue found by addrss sanitizer If an attribute had an empty value (which is apparently possible with some tokens), the copy function would fail to initialize the pointer causing an invalid free when the attributes array is freed. Ensure pValue is zeroed on copy if the attri bute length is 0, and also ensure the attribute length is correctly set to 0. Then belt&suspender approach ensure the buffer is allocated with OPENSSL_zalloc instead of OPENSSL_malloc, which will eansure by default all addresses and legth are a safe default NULL/0 value. Signed-off-by: Simo Sorce --- src/objects.c | 2 +- src/util.c | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/objects.c b/src/objects.c index d0f97f04..e4519fa1 100644 --- a/src/objects.c +++ b/src/objects.c @@ -2762,7 +2762,7 @@ static CK_RV return_dup_key(P11PROV_OBJ *dst, P11PROV_OBJ *src) dst->cka_token = src->cka_token; dst->data.key = src->data.key; - dst->attrs = OPENSSL_malloc(sizeof(CK_ATTRIBUTE) * src->numattrs); + dst->attrs = OPENSSL_zalloc(sizeof(CK_ATTRIBUTE) * src->numattrs); if (!dst->attrs) { rv = CKR_HOST_MEMORY; P11PROV_raise(dst->ctx, rv, "Failed allocation"); diff --git a/src/util.c b/src/util.c index 59f52796..a6140f8f 100644 --- a/src/util.c +++ b/src/util.c @@ -1016,8 +1016,10 @@ CK_RV p11prov_copy_attr(CK_ATTRIBUTE *dst, CK_ATTRIBUTE *src) return CKR_HOST_MEMORY; } memcpy(dst->pValue, src->pValue, src->ulValueLen); - dst->ulValueLen = src->ulValueLen; + } else { + dst->pValue = NULL; } + dst->ulValueLen = src->ulValueLen; dst->type = src->type; return CKR_OK;