Skip to content

Commit

Permalink
Fix issue found by addrss sanitizer
Browse files Browse the repository at this point in the history
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 <simo@redhat.com>
  • Loading branch information
simo5 committed Aug 1, 2024
1 parent 5e100d4 commit 02cb59f
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/objects.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
4 changes: 3 additions & 1 deletion src/util.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 02cb59f

Please sign in to comment.