Closed
Bug 1215944
Opened 9 years ago
Closed 9 years ago
crash in nsStandardURL::BuildNormalizedSpec
Categories
(Core :: Networking, defect)
Tracking
()
RESOLVED
FIXED
mozilla44
People
(Reporter: gfritzsche, Assigned: valentin)
References
Details
(4 keywords, Whiteboard: [post-critsmash-triage][adv-main42-][adv-esr38.4-])
Crash Data
Attachments
(2 files)
738 bytes,
patch
|
mcmanus
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
ritu
:
approval-mozilla-esr38+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
1.26 KB,
patch
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-2d06acbd-2ff0-464e-bb3a-f745b2151018. ============================================================= This was consistently reproducible over multiple tries and Fx restarts by just opening a new tab and CMD-V pasting a https:// URL that was inserted into the clipboard by Dropbox. After a few restarts this suddenly stopped crashing, but i could potentially try around in case the stack and/or dumps are not sufficient. Making it security-sensitive until it is clear whether this is exploitable. Setting needinfo per hg blame information around the crash location. Examples: bp-880147bb-1bb9-48f7-bc7d-5fc2c2151018 bp-2d06acbd-2ff0-464e-bb3a-f745b2151018 bp-8f293365-482f-4b14-b0e7-fdb5d2151018 bp-873504c3-4ecb-47c7-96a9-dd3d22151018 bp-71828d78-0983-412d-a211-7e8102151018
Flags: needinfo?
Reporter | ||
Updated•9 years ago
|
Flags: needinfo?(valentin.gosu)
Flags: needinfo?(mcmanus)
Flags: needinfo?
Assignee | ||
Comment 1•9 years ago
|
||
Thanks Georg, I've identified the bug. Could you provide the URL you pasted so we could have a reliable test case? Thanks!
Assignee | ||
Comment 2•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7b7672cc4368
Updated•9 years ago
|
Flags: needinfo?(mcmanus)
Assignee | ||
Comment 3•9 years ago
|
||
This reintroduces the empty string check I had mistakenly removed when I added the length parameter in bug 1199430
Assignee: nobody → valentin.gosu
Attachment #8675505 -
Flags: review?(mcmanus)
Reporter | ||
Comment 4•9 years ago
|
||
(In reply to Valentin Gosu [:valentin] from comment #1) > Thanks Georg, > I've identified the bug. Could you provide the URL you pasted so we could > have a reliable test case? > Thanks! This was the URL i was trying to copy-paste: https://www.dropbox.com/s/5j4p5mpodv3pie3/sektore_v3.mp3?dl=0 However, this might also have been Dropbox putting unexpected contents in the clipboard as sometimes i got an an empty string trying to paste elsewhere. The best thing would be to check the submitted dumps for what data was actually passed here.
Updated•9 years ago
|
Attachment #8675505 -
Flags: review?(mcmanus) → review+
Updated•9 years ago
|
Keywords: regression
Assignee | ||
Comment 5•9 years ago
|
||
Comment on attachment 8675505 [details] [diff] [review] bug1215944-validhostname-empty.patch [Security approval request comment] How easily could an exploit be constructed based on the patch? Very difficult. The crash is caused by an out-of-bounds read when the length of the string is 0. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No. Which older supported branches are affected by this flaw? Present in aurora, beta and esr38 (not in release) If not all supported branches, which bug introduced the flaw? Introduced in bug 1199430 which was uplifted to beta, aurora and esr38. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Yes. The patch is very simple. How likely is this patch to cause regressions; how much testing does it need? This shouldn't cause any regressions as this check was there before 1199430 but I mistakenly removed it.
Attachment #8675505 -
Flags: sec-approval?
Comment 6•9 years ago
|
||
Comment on attachment 8675505 [details] [diff] [review] bug1215944-validhostname-empty.patch sec-approval+ for trunk. This needs nominated patches for Aurora, Beta, and ESR38. Sooner is better than later!
Attachment #8675505 -
Flags: sec-approval? → sec-approval+
Updated•9 years ago
|
status-firefox41:
--- → unaffected
status-firefox42:
--- → affected
status-firefox43:
--- → affected
status-firefox-esr38:
--- → affected
tracking-firefox42:
--- → +
tracking-firefox43:
--- → +
tracking-firefox-esr38:
--- → 42+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 7•9 years ago
|
||
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8675505 [details] [diff] [review] bug1215944-validhostname-empty.patch Approval Request Comment [Feature/regressing bug #]: Bug 1199430 [User impact if declined]: Possible crashes when loading ignored-single-character-IDNA URLs or other single character hostname URLs. [Describe test coverage new/current, TreeHerder]: Attached test does not cause a crash after this patch is applied. [Risks and why]: Low risk, as this check was there before bug 1199430. [String/UUID change made/needed]: None
Attachment #8675505 -
Flags: approval-mozilla-esr38?
Attachment #8675505 -
Flags: approval-mozilla-beta?
Attachment #8675505 -
Flags: approval-mozilla-aurora?
Comment 9•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8dc73ca2ec85
Keywords: checkin-needed
Updated•9 years ago
|
Group: core-security → network-core-security
https://hg.mozilla.org/mozilla-central/rev/8dc73ca2ec85
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Updated•9 years ago
|
Group: network-core-security → core-security-release
Comment 13•9 years ago
|
||
Comment on attachment 8675505 [details] [diff] [review] bug1215944-validhostname-empty.patch Seems safe and fixes a crash, taking it. Should be in 42 beta 9.
Attachment #8675505 -
Flags: approval-mozilla-beta?
Attachment #8675505 -
Flags: approval-mozilla-beta+
Attachment #8675505 -
Flags: approval-mozilla-aurora?
Attachment #8675505 -
Flags: approval-mozilla-aurora+
Comment 14•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/4c75c3607267 https://hg.mozilla.org/releases/mozilla-beta/rev/50213377c223
Comment 15•9 years ago
|
||
Georg, if you have a minute, would you want to take a look at this to confirm that it was fixed? Thank you.
Flags: needinfo?(gfritzsche)
Comment on attachment 8675505 [details] [diff] [review] bug1215944-validhostname-empty.patch Taking it for esr38.4.0 as it's a sec-high and a simple fix.
Attachment #8675505 -
Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
Reporter | ||
Comment 18•9 years ago
|
||
(In reply to Matt Wobensmith from comment #15) > Georg, if you have a minute, would you want to take a look at this to > confirm that it was fixed? Thank you. I'm not able to reproduce this right now.
Flags: needinfo?(gfritzsche)
Updated•9 years ago
|
Whiteboard: [post-critsmash-triage]
Updated•9 years ago
|
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main42-][adv-esr38.4-]
Updated•9 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•