Closed Bug 1215944 Opened 5 years ago Closed 5 years ago
crash in ns
Standard URL::Build Normalized Spec
738 bytes, patch
|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
Thanks Georg, I've identified the bug. Could you provide the URL you pasted so we could have a reliable test case? Thanks!
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)
(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.
Attachment #8675505 - Flags: review?(mcmanus) → review+
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 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+
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
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.
Georg, if you have a minute, would you want to take a look at this to confirm that it was fixed? Thank you.
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+
(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.
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main42-][adv-esr38.4-]
You need to log in before you can comment on or make changes to this bug.