Closed
Bug 734007
Opened 12 years ago
Closed 11 years ago
sizeof(SSL3_MASTER_SECRET_LENGTH) used instead of SSL3_MASTER_SECRET_LENGTH in ssl3_ServerHandleSessionTicketXtn
Categories
(NSS :: Libraries, defect, P2)
NSS
Libraries
Tracking
(Not tracked)
RESOLVED
FIXED
3.15.2
People
(Reporter: briansmith, Assigned: wtc)
Details
Attachments
(1 file)
1.08 KB,
patch
|
ryan.sleevi
:
review+
wtc
:
checked-in+
|
Details | Diff | Splinter Review |
The code is: /* Allow for the wrapped master secret to be longer. */ if (buffer_len < sizeof(SSL3_MASTER_SECRET_LENGTH)) goto no_ticket; it should be: /* Allow for the wrapped master secret to be longer. */ if (buffer_len < SSL3_MASTER_SECRET_LENGTH) goto no_ticket;
Assignee | ||
Comment 1•12 years ago
|
||
r=wtc. After staring at that code for several minutes, I think that test can be weaker: if (buffer_len < parsed_session_ticket->ms_length) goto no_ticket; This weaker test ensures that 'buffer' has at least parsed_session_ticket->ms_length bytes, otherwise the subsequent PORT_Memcpy call will read beyond the end of 'buffer'.
Comment 2•11 years ago
|
||
Changes if (buffer_len < sizeof(SSL3_MASTER_SECRET_LENGTH)) to if (buffer_len < parsed_session_ticket->ms_length) as described in Comment 1.
Attachment #793746 -
Flags: review?(wtc)
Assignee | ||
Comment 3•11 years ago
|
||
Comment on attachment 793746 [details] [diff] [review] Proposed Patch I can't review this patch because I suggested this change. Ryan, I remember this bug was reported to you independently. Could you review this patch? Thanks.
Attachment #793746 -
Flags: review?(wtc) → review?(ryan.sleevi)
Comment 4•11 years ago
|
||
Comment on attachment 793746 [details] [diff] [review] Proposed Patch Review of attachment 793746 [details] [diff] [review]: ----------------------------------------------------------------- Dang it, you're right Wan-Teh. I thought I landed that fix too. r+ though, because this is clearly correct (especially with your change).
Attachment #793746 -
Flags: review?(ryan.sleevi) → review+
Assignee | ||
Comment 5•11 years ago
|
||
Comment on attachment 793746 [details] [diff] [review] Proposed Patch https://hg.mozilla.org/projects/nss/rev/2ab64817f703
Attachment #793746 -
Flags: checked-in+
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → wtc
Status: NEW → RESOLVED
Closed: 11 years ago
Priority: -- → P2
Resolution: --- → FIXED
Target Milestone: --- → 3.15.2
You need to log in
before you can comment on or make changes to this bug.
Description
•