21 Apr
2021
21 Apr
'21
4:53 p.m.
Hello,
The macro MBEDTLS_MPI_CHK sets ret, so this particular case is safe.
That being said, we do have a hygiene rule to initialize ret variables, to avoid accidentally having uninitialized variables in edge cases. I'll file an issue to fix those.
Thanks for reaching out!
--
Gilles Peskine
Mbed TLS developer
On 21/04/2021 17:33, momo 19 via mbed-tls wrote:
> Hello,
>
> I would like to report a possible bug in rsa_prepare_blinding function
> in rsa.c
> (https://github.com/ARMmbed/mbedtls/blob/v2.26.0/library/rsa.c
> https://github.com/ARMmbed/mbedtls/blob/v2.26.0/library/rsa.c). I am
> not sure if it is a real issue, but I think that there is a
> possibility to use uninitialized variable ret:
>
> static int rsa_prepare_blinding( mbedtls_rsa_context *ctx,
> int (*f_rng)(void *, unsigned char *, size_t), void
> *p_rng )
> {
> int ret, count = 0; <--- uninitialized variable ret
> mbedtls_mpi R;
>
> mbedtls_mpi_init( &R );
>
> if( ctx->Vf.p != NULL )
> {
> /* We already have blinding values, just update them by
> squaring */
> MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &ctx->Vi, &ctx->Vi,
> &ctx->Vi ) );
> MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &ctx->Vi, &ctx->Vi,
> &ctx->N ) );
> MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &ctx->Vf, &ctx->Vf,
> &ctx->Vf ) );
> MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &ctx->Vf, &ctx->Vf,
> &ctx->N ) );
>
> goto cleanup; <--- going to cleanup without setting a value of ret
> }
>
> (Skipping lines for readability)
>
> cleanup:
> mbedtls_mpi_free( &R );
>
> return( ret ); <--- returning uninitialized variable ret
> }
>
> Best regards,
> grapix121
>
>