Closed Bug 903565 Opened 12 years ago Closed 12 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+
Status: ASSIGNED → RESOLVED
Closed: 12 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.

Attachment

General

Creator:
Created:
Updated:
Size: