[Skiboot] [PATCH v2 5/5] secvar/pkcs7: fix a wrong sizeof()

Nayna nayna at linux.vnet.ibm.com
Tue Jun 22 11:31:00 AEST 2021


On 6/21/21 4:26 AM, Daniel Axtens wrote:
> This code isn't directly used by skiboot, but it is wrong and potentially
> insecure so I'm fixing it in case it's used in the future.
>
> We pass sizeof(hash) into mbedtls_pk_verify(). However, hash is a pointer,
> not an array, so rather than passing the length of the hash to verify we'll
> pass in 8, and only compare the first 8 bytes of the hash rather than all 32.
>
> Pass in 0 instead. That tells mbedtls to work out the length based on the
> hash type. We allocated enough memory for whatever hash type the PKCS#7
> message declared so this will be safe.
>
> Signed-off-by: Daniel Axtens <dja at axtens.net>
> ---
>   libstb/crypto/pkcs7/pkcs7.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libstb/crypto/pkcs7/pkcs7.c b/libstb/crypto/pkcs7/pkcs7.c
> index 4407e201a4cc..3f41ba7acb2e 100644
> --- a/libstb/crypto/pkcs7/pkcs7.c
> +++ b/libstb/crypto/pkcs7/pkcs7.c
> @@ -538,7 +538,7 @@ int mbedtls_pkcs7_signed_data_verify( mbedtls_pkcs7 *pkcs7,
>   
>       mbedtls_md( md_info, data, datalen, hash );
>   
> -    ret = mbedtls_pk_verify( &pk_cxt, md_alg, hash, sizeof(hash),
> +    ret = mbedtls_pk_verify( &pk_cxt, md_alg, hash, 0,

Just like other function why do not we pass here also hashlen which can 
be calculated by mbedtls_md_get_size( md_info ) ?

Thanks & Regards,

     - Nayna



More information about the Skiboot mailing list