Closed Bug 1288482 (CVE-2016-5292) Opened 4 years ago Closed 4 years ago
URL parsing causes crash
7.33 KB, text/plain
93 bytes, text/html
7.74 KB, patch
|Details | Diff | Splinter Review|
7.72 KB, patch
|Details | Diff | Splinter Review|
+++ This bug was initially created as a clone of Bug #1288123 +++ NOTE: This clone was created because the description of the initial bug was crashing browsers. The original was logged by firstname.lastname@example.org. WARNING: Opening the Bug #1288123 may trigger crash. User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36 Steps to reproduce: Create an html page with the following contents, access it, then "reload this page" one time. Actual results: It crashes for version 46+ on OS X, including FirefoxNightly 50.0a1 (2016-07-15). Older versions and other platforms work. (45 on OS X works, 47 on Linux works, etc.) Process: firefox  Path: /Applications/NightlyDebug.app/Contents/MacOS/firefox Identifier: org.mozilla.nightlydebug Version: 47.0.2 (4716.7.2) Code Type: X86-64 (Native) Parent Process: ???  Responsible: firefox  User ID: 25628395 Date/Time: 2016-07-20 08:00:17.778 -0700 OS Version: Mac OS X 10.11.5 (15F34) Report Version: 11 Anonymous UUID: 4F065409-6410-EED7-4352-470B9B7EA9DD Sleep/Wake UUID: 1B10C1A4-591F-4DAC-AD77-30B47ECCF792 Time Awake Since Boot: 400000 seconds Time Since Wake: 2900 seconds System Integrity Protection: enabled Crashed Thread: 0 Dispatch queue: com.apple.main-thread Exception Type: EXC_BAD_ACCESS (SIGSEGV) Exception Codes: KERN_INVALID_ADDRESS at 0x0000000213393047 Exception Note: EXC_CORPSE_NOTIFY VM Regions Near 0x213393047: mapped file 000000019de00000-000000019e082000 [ 2568K] r--/rwx SM=COW --> STACK GUARD 0000700000000000-0000700000001000 [ 4K] ---/rwx SM=NUL stack guard for thread 62 Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 libsystem_platform.dylib 0x00007fff83382360 _platform_strncmp + 320 1 XUL 0x00000001052a5db7 0x105093000 + 2174391 2 XUL 0x00000001086e0f64 0x105093000 + 56942436 3 XUL 0x00000001051b695a 0x105093000 + 1194330 4 XUL 0x00000001051b1b9d 0x105093000 + 1174429 5 XUL 0x000000010866a5ae 0x105093000 + 56456622 6 XUL 0x000000010623009f 0x105093000 + 18469023 7 XUL 0x00000001062fb175 0x105093000 + 19300725 [...] Expected results: No crash.
Valentin, could you look at this? The top of the stack at least is in a strcmp of some URL thing, I believe.
Members of the triage team observed a crash in Nightly (50) on Mac and Linux, but not on Windows. No crash on Dev Edition (49). Since it appears to be a bad access from strcmp() whether it crashes may depend on build differences and what happens to be allocated at the incorrect pointer. Testing in an ASAN build may reveal this to go back much much further than the reporter's 46+ (though we don't have to care about anything earlier than ESR-45 at this point).
The issue seems to have been introduced by bug 1042347. It happens because nsStandardURL::CoalescePath is expecting net_CoalesceDirs to only normalize the directory, not the entire filepath - which screws up the offsets.
Assignee: nobody → valentin.gosu
Attachment #8774154 - Flags: review?(mcmanus)
The fix for this was getting to be quite complicated - something I wouldn't feel confident to uplift. So I think the right decision is to backout bug 1042347, and fix this corner case in that bug.
Comment on attachment 8774154 [details] [diff] [review] Backout bug 1042347 Review of attachment 8774154 [details] [diff] [review]: ----------------------------------------------------------------- just to note that this isn't a straight backout of https://hg.mozilla.org/mozilla-central/rev/f3925f5ffdbb because of a change in test coverage.. not sure if that will impact cleanliness of uplifts.
Attachment #8774154 - Flags: review?(mcmanus) → review+
Comment on attachment 8774154 [details] [diff] [review] Backout bug 1042347 [Security approval request comment] How easily could an exploit be constructed based on the patch? Not easily. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No, but bugs 1288481, 1288123, 1288481 are public and contain examples of URLs that crash the browsers. Which older supported branches are affected by this flaw? Starting with Mozilla 47 If not all supported branches, which bug introduced the flaw? Bug 1042347. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Easy to backport. Only involves a backout of bug 1042347. How likely is this patch to cause regressions; how much testing does it need? Unlikely. Only a backout.
Attachment #8774154 - Flags: sec-approval?
I'm giving this sec-approval+ for trunk checkin now since the URLs are visible in bugs.
Comment on attachment 8774154 [details] [diff] [review] Backout bug 1042347 We'll want patches for other affected branches made and nominated after this is on trunk.
Attachment #8774154 - Flags: sec-approval? → sec-approval+
Comment on attachment 8776409 [details] [diff] [review] Backout bug 1042347 [aurora] Approval Request Comment [Feature/regressing bug #]: Bug 1042347 [User impact if declined]: Crash with specially crafter URL. Security risk. [Describe test coverage new/current, TreeHerder]: Some unit tests. Previous handling of %2e is known to be safe. [Risks and why]: Low. This is just a backout. [String/UUID change made/needed]: None.
Attachment #8776409 - Flags: approval-mozilla-aurora?
Comment on attachment 8776409 [details] [diff] [review] Backout bug 1042347 [aurora] Fix a crash and sec-high, taking it to aurora.
Attachment #8776409 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [necko-active] → [necko-active][post-critsmash-triage]
Whiteboard: [necko-active][post-critsmash-triage] → [necko-active][post-critsmash-triage][adv-main50+]
You need to log in before you can comment on or make changes to this bug.