1258715 - PK11_GenerateRandom return values in ssl3con

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P3)

Description

[Franziskus Kiefer [:franziskus]]

the return value of the first PK11_GenerateRandom is assign but not checked. The second PK11_GenerateRandom is directly ignoring the return value.

>   if (client_version != ss->clientHelloVersion) {
>     /* Version roll-back detected. ensure failure.  */
>     rv = PK11_GenerateRandom(rsaPmsBuf, sizeof rsaPmsBuf);
>   }
> }
> /* have PMS, build MS without PKCS11 */
> rv = ssl3_MasterSecretDeriveBypass(pwSpec, cr, sr, &pmsItem, isTLS, PR_TRUE);
> if (rv != SECSuccess) {
>   pwSpec->msItem.data = pwSpec->raw_master_secret;
>   pwSpec->msItem.len = SSL3_MASTER_SECRET_LENGTH;
>   PK11_GenerateRandom(pwSpec->msItem.data, pwSpec->msItem.len);
> }

Comment 1

[Manish [:manishkk]]

Seems someone resolved this issue!
https://searchfox.org/mozilla-central/source/security/nss/lib/ssl/ssl3con.c#1021


Please confirm!

(In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #0)
> the return value of the first PK11_GenerateRandom is assign but not checked.
> The second PK11_GenerateRandom is directly ignoring the return value.
> 
> >   if (client_version != ss->clientHelloVersion) {
> >     /* Version roll-back detected. ensure failure.  */
> >     rv = PK11_GenerateRandom(rsaPmsBuf, sizeof rsaPmsBuf);
> >   }
> > }
> > /* have PMS, build MS without PKCS11 */
> > rv = ssl3_MasterSecretDeriveBypass(pwSpec, cr, sr, &pmsItem, isTLS, PR_TRUE);
> > if (rv != SECSuccess) {
> >   pwSpec->msItem.data = pwSpec->raw_master_secret;
> >   pwSpec->msItem.len = SSL3_MASTER_SECRET_LENGTH;
> >   PK11_GenerateRandom(pwSpec->msItem.data, pwSpec->msItem.len);
> > }

Comment 2

[Franziskus Kiefer [:franziskus]]

Yes this was fixed as part of another patch indeed. Thanks for spotting!