libckteec: fix integer overflow in PKCS#11 serializer#415
Conversation
| { | ||
| size_t nlen = *blen + len; | ||
| char *buf = realloc(*bstart, nlen); | ||
| char *buf; |
There was a problem hiding this comment.
Please initialize it to NULL here.
There was a problem hiding this comment.
Done — initialized buf to NULL, updated SoB to my real name, and added a Fixes tag.
|
Please use your real name in the SoB as described in https://optee.readthedocs.io/en/latest/general/contribute.html#developer-certificate-of-origin It would also be nice with a Fixes: tag pointing at the commit that introduced the bug. |
bestminhal
force-pushed
the
fix/pkcs11-serializer-integer-overflow
branch
from
dfcadc5 to
8e14cce
Compare
|
Looks good. @etienne-lms? |
etienne-lms
left a comment
etienne-lms
left a comment
There was a problem hiding this comment.
LGTM aside minor comments. Thanks for the fixes.
| if (nlen < *blen) | ||
| return CKR_HOST_MEMORY; | ||
|
|
||
| buf = realloc(*bstart, nlen); |
There was a problem hiding this comment.
Nitpicking: could remove the empty line below.
| char *buf = NULL; | ||
|
|
||
| if (nlen < *blen) | ||
| return CKR_HOST_MEMORY; |
There was a problem hiding this comment.
I think CKR_ARGUMENTS_BAD would better apply here.
|
Good points, both fixed. |
|
This pull request has been marked as a stale pull request because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this pull request will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time. |
|
Looks good, please squash and apply: |
Add overflow check in serialize() to prevent size_t wraparound when computing new buffer length. A crafted ulValueLen could cause *blen + len to wrap, leading to a small realloc followed by an out-of-bounds memcpy. Also add a bounds check in serialize_ck_attribute() for the CKA_ALLOWED_MECHANISMS path where n * sizeof(uint32_t) could overflow the uint32_t pkcs11_size, resulting in an undersized malloc. Fixes: 85a7ea7 ("libckteec: introduce helpers for serializing data") Signed-off-by: Minghao Cheng <m@minhal.me> Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
bestminhal
force-pushed
the
fix/pkcs11-serializer-integer-overflow
branch
from
439a263 to
6ecbbf1
Compare
3 participants
Fix integer overflow in serialize() and serialize_ck_attribute() that could lead to heap buffer overflow via crafted PKCS#11 attributes.