ta: crypt: fix out-of-bounds read in oversized extra data test #806
Conversation
| */ | ||
| res = derive_unique_key(session, key1, sizeof(key1), extra_key_data, | ||
| res = derive_unique_key(session, key1, sizeof(key1), | ||
| extra_key_data_large, |
There was a problem hiding this comment.
I see why the buffer must be large, but I don't see the point with this particular test or even why the PTA must enforce the limit TA_DERIVED_EXTRA_DATA_MAX_SIZE.
@jockebech do you remember?
|
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 pull request at any time. |
jenswi-linaro
left a comment
jenswi-linaro
left a comment
There was a problem hiding this comment.
With my comment addressed, please apply:
Reviewed-by: Jens Wiklander <[email protected]>
ta/crypt/derive_key_taf.c
Outdated
| TEE_MemFill(key2, 0, sizeof(key2)); | ||
|
|
||
| /* | ||
| * Testing limits for extra data size (if this would success, then we |
There was a problem hiding this comment.
Please update this comment. I think removing the "(...)" should be the best.
derive_ta_unique_key_test() was calling derive_unique_key() with extra_data_len set to TA_DERIVED_EXTRA_DATA_MAX_SIZE + 1 while passing a much smaller extra_key_data buffer. map_tmp_param() used the length as-is in memcpy(), which resulted in an out-of-bounds read from the source buffer. Signed-off-by: Aleksandr Iashchenko <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>
2 participants
This change fixes an out-of-bounds read in the
derive_ta_unique_key_test()oversized extra data test.The test was calling
derive_unique_key()withextra_data_lenset toTA_DERIVED_EXTRA_DATA_MAX_SIZE + 1while passing a much smallerextra_key_databuffer on the stack. Inderive_unique_key(),map_tmp_param()usedextra_data_lenas thememcpy()size, which caused an out-of-bounds read from the source buffer.The fix is to provide a dedicated
extra_key_data_largebuffer sized toTA_DERIVED_EXTRA_DATA_MAX_SIZE + 1and use it for the oversized extra data test.