Uninitialized variable used in ssl3_HandleCertificate when sending alert about missing certificate

RESOLVED FIXED in 3.14.2

Status

NSS
Libraries
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: briansmith, Assigned: mcmanus)

Tracking

(Blocks: 1 bug)

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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;|
(Assignee)

Comment 1

5 years ago
Created attachment 703712 [details] [diff] [review]
patch 0
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
Last Resolved: 5 years ago
Resolution: --- → FIXED
Blocks: 832432

Updated

5 years ago
Attachment #703712 - Flags: review+
You need to log in before you can comment on or make changes to this bug.