sizeof(SSL3_MASTER_SECRET_LENGTH) used instead of SSL3_MASTER_SECRET_LENGTH in ssl3_ServerHandleSessionTicketXtn
RESOLVED
FIXED
in 3.15.2
Status
People
(Reporter: briansmith, Assigned: wtc)
Tracking
Firefox Tracking Flags
(Not tracked)
Details
Attachments
(1 attachment)
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•7 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•6 years ago
|
||
Created attachment 793746 [details] [diff] [review] Proposed Patch 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•6 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•6 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•6 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•6 years ago
|
Assignee: nobody → wtc
Status: NEW → RESOLVED
Last Resolved: 6 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
•