Closed
Bug 903565
Opened 12 years ago
Closed 12 years ago
Crash in ssl_NewSocket in failure case
Categories
(NSS :: Libraries, defect, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
3.15.2
People
(Reporter: ryan.sleevi, Assigned: ryan.sleevi)
Details
Attachments
(1 file)
1.03 KB,
patch
|
wtc
:
review+
ryan.sleevi
:
checked-in+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #788315 -
Flags: review?(wtc)
Comment 2•12 years ago
|
||
Comment on attachment 788315 [details] [diff] [review]
Fix for protocolVariant initialization
r=wtc. Sorry about the bug!
Attachment #788315 -
Flags: review?(wtc) → review+
Assignee | ||
Comment 3•12 years ago
|
||
Committed as https://hg.mozilla.org/projects/nss/rev/de33c29a6bde
Fixed in 3.15.2
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•12 years ago
|
Attachment #788315 -
Flags: checked-in+
Updated•12 years ago
|
Priority: -- → P2
You need to log in
before you can comment on or make changes to this bug.
Description
•