On Sunday 19 August 2018 08:44:24 Theo Buehler wrote: > Coverity complains about the case where EVP_Digest() fails, but there > are a couple more.
One thing worth mentioning... previously it would return -1 without setting an error, whereas now it will always set RSA_R_OAEP_DECODING_ERROR (even if the underlying failure was something unrelated like a malloc failure). I'm not overly concerned by this, but we could (if we wanted) keep the previous behaviour by adding an 'err' label that jumps to the 'free(db);' line and use that for non-decoding failures. Either way, ok jsing@ > Index: rsa/rsa_oaep.c > =================================================================== > RCS file: /var/cvs/src/lib/libcrypto/rsa/rsa_oaep.c,v > retrieving revision 1.27 > diff -u -p -r1.27 rsa_oaep.c > --- rsa/rsa_oaep.c 5 Aug 2018 13:30:04 -0000 1.27 > +++ rsa/rsa_oaep.c 19 Aug 2018 06:38:52 -0000 > @@ -126,8 +126,7 @@ RSA_padding_check_PKCS1_OAEP(unsigned ch > } > > dblen = num - SHA_DIGEST_LENGTH; > - db = malloc(dblen + num); > - if (db == NULL) { > + if ((db = malloc(dblen + num)) == NULL) { > RSAerror(ERR_R_MALLOC_FAILURE); > return -1; > } > @@ -143,17 +142,17 @@ RSA_padding_check_PKCS1_OAEP(unsigned ch > maskeddb = padded_from + SHA_DIGEST_LENGTH; > > if (MGF1(seed, SHA_DIGEST_LENGTH, maskeddb, dblen)) > - return -1; > + goto decoding_err; > for (i = 0; i < SHA_DIGEST_LENGTH; i++) > seed[i] ^= padded_from[i]; > > if (MGF1(db, dblen, seed, SHA_DIGEST_LENGTH)) > - return -1; > + goto decoding_err; > for (i = 0; i < dblen; i++) > db[i] ^= maskeddb[i]; > > if (!EVP_Digest((void *)param, plen, phash, NULL, EVP_sha1(), NULL)) > - return -1; > + goto decoding_err; > > if (timingsafe_memcmp(db, phash, SHA_DIGEST_LENGTH) != 0 || bad) > goto decoding_err; > @@ -177,7 +176,7 @@ RSA_padding_check_PKCS1_OAEP(unsigned ch > free(db); > return mlen; > > -decoding_err: > + decoding_err: > /* > * To avoid chosen ciphertext attacks, the error message should not > * reveal which kind of decoding error happened