Closed Bug 832005 Opened 9 years ago Closed 9 years ago

Uninitialized variable used in ssl3_HandleCertificate when sending alert about missing certificate

Categories

(NSS :: Libraries, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
3.14.2

People

(Reporter: briansmith, Assigned: mcmanus)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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;|
Attached patch patch 0Splinter Review
Attachment #703712 - Flags: review?(bsmith)
Attachment #703712 - Flags: review?(bsmith) → review+
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
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Attachment #703712 - Flags: review+
You need to log in before you can comment on or make changes to this bug.