Closed Bug 903565 Opened 9 years ago Closed 9 years ago

Crash in ssl_NewSocket in failure case

Categories

(NSS :: Libraries, defect, P2)

3.14
defect

Tracking

(Not tracked)

RESOLVED FIXED
3.15.2

People

(Reporter: ryan.sleevi, Assigned: ryan.sleevi)

Details

Attachments

(1 file)

Bug 681065 had a merge issue when landing, such that the socket protocol variant was assigned after all the other fields were initialized. However, the assignment of this field was included in the loser: case, which means it may cause a NULL memory dereference (and thus, a crash) if initializing the socket failed.

This was reported in Chromium as http://crbug.com/270735, with the sample stacktrace reproduced below:

The problem is that |ss| is used after it is reset to NULL:

	if (makeLocks) {
	    status = ssl_MakeLocks(ss);
	    if (status != SECSuccess)
		goto loser;                          // Will crash below
	}
	status = ssl_CreateSecurityInfo(ss);
	if (status != SECSuccess) 
	    goto loser;                              // Will crash below
	status = ssl_InitGather(&ss->gs);
	if (status != SECSuccess) {
loser:                                               // Will crash below
	    ssl_DestroySocketContents(ss);
	    ssl_DestroyLocks(ss);
	    PORT_Free(ss);
	    ss = NULL;                               // <-- clearing |ss|
	}
	ss->protocolVariant = protocolVariant;       // <-- CRASH
    }
    return ss;
}

0x62b26210	 [remoting_core.dll]	 - sslsock.c:3124]	ssl_NewSocket
0x62b26d12	 [remoting_core.dll]	 - sslsock.c:1415]	ssl_ImportFD
0x62b26d98	 [remoting_core.dll]	 - sslsock.c:1447]	SSL_ImportFD
Attachment #788315 - Flags: review?(wtc)
Comment on attachment 788315 [details] [diff] [review]
Fix for protocolVariant initialization

r=wtc. Sorry about the bug!
Attachment #788315 - Flags: review?(wtc) → review+
Committed as https://hg.mozilla.org/projects/nss/rev/de33c29a6bde

Fixed in 3.15.2
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Attachment #788315 - Flags: checked-in+
Priority: -- → P2
You need to log in before you can comment on or make changes to this bug.