311590 - Uninitialized variable 'slot' in ssl3_HandleServerHello in ssl3con.c

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Wan-Teh Chang]

When compiling revision 1.75 of ssl3con.c, I get this compiler
warning:

c:\nss-tip\mozilla\security\nss\lib\ssl\ssl3con.c(4254) : warning C4700: local v
ariable 'slot' used without having been initialized

Comment 1

[Julien Pierre]

This call is part of a block of code that says :
	    /* We CAN restart a bypass session in a non-bypass socket. */
	    /* need to import the raw master secret to session object */

This code path has likely never been tested.
The compiler is correct to warn, and the program will crash on this PK11_FreeSlot.

IMO, we should either create a test case for this code path and add it to all.sh, or remove it altogether, as I'm not sure it is needed - we can probably get away with doing a full handshake instead of a restart.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v1

The erroneous code in ssl3_HandleServerHello is almost identical to another 
block of code in ssl3_HandleClientHello, which works.  So, this patch changes
the former client code to behave the same as, and more closely resemble, the 
later server code.

Comment 3

[Wan-Teh Chang]

Comment on attachment 203342 [details] [diff] [review]
patch v1

Nelson, did you compile this code?  'slot' is
now only declared in one of the blocks that use
it.

Comment 4

[Wan-Teh Chang]

Comment on attachment 203342 [details] [diff] [review]
patch v1

> 	} else {
> 	    /* We CAN restart a bypass session in a non-bypass socket. */
> 	    /* need to import the raw master secret to session object */
>+	    PK11SlotInfo *slot = PK11_GetInternalSlot();

Sorry, I thought this is an assignment statement, but
it is a declaration with an initializer.  r=wtc.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

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