832005 - Uninitialized variable used in ssl3_HandleCertificate when sending alert about missing certificate

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

When building NSS with clang:

> ssl3con.c:8390:6: warning: variable 'desc' is used uninitialized
> whenever 'if' condition is true [-Wsometimes-uninitialized]
>         if (!(isTLS && isServer))
>             ^~~~~~~~~~~~~~~~~~~~
> ssl3con.c:8602:43: note: uninitialized use occurs here
>     (void)SSL3_SendAlert(ss, alert_fatal, desc);
>                                           ^~~~
> ssl3con.c:8390:2: note: remove the 'if' if its condition is always
> false
>         if (!(isTLS && isServer))
>         ^~~~~~~~~~~~~~~~~~~~~~~~~
> ssl3con.c:8349:5: note: variable 'desc' is declared here
>     SSL3AlertDescription desc;
>     ^


> it probably needs something like
> diff --git a/security/nss/lib/ssl/ssl3con.c
> b/security/nss/lib/ssl/ssl3con.c
> --- a/security/nss/lib/ssl/ssl3con.c
> +++ b/security/nss/lib/ssl/ssl3con.c
> @@ -8380,18 +8380,20 @@ ssl3_HandleCertificate(sslSocket *ss, SS
> remaining = ssl3_ConsumeHandshakeNumber(ss, 3, &b, &length);
> if (remaining < 0)
> goto loser;	/* fatal alert already sent by ConsumeHandshake. */
> if ((PRUint32)remaining > length)
> goto decode_loser;
> }
> 
> if (!remaining) {
> -	if (!(isTLS && isServer))
> +	if (!(isTLS && isServer)) {
> +	    desc = internal_error;
> goto alert_loser;
> +	}
>
> please let me know if that's the right fix and I'll open a bug/patch
> to make the review cycle easier

The suggested fix is correct except it should be |desc = bad_certificate;|

Comment 1

[Patrick McManus [:mcmanus]]

Attachment: patch 0

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Checking in lib/ssl/ssl3con.c;
/cvsroot/mozilla/security/nss/lib/ssl/ssl3con.c,v  <--  ssl3con.c
new revision: 1.197; previous revision: 1.196
done