Closed Bug 1288482 (CVE-2016-5292) Opened 4 years ago Closed 4 years ago

URL parsing causes crash

Categories

(Core :: Networking, defect, critical)

50 Branch
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox47 --- wontfix
firefox48 --- wontfix
firefox49 --- affected
firefox-esr45 --- unaffected
firefox50 --- fixed

People

(Reporter: tsmith, Assigned: valentin)

References

Details

(Keywords: sec-high, testcase, Whiteboard: [necko-active][post-critsmash-triage][adv-main50+])

Attachments

(4 files, 1 obsolete file)

Attached file linux_asan_log.txt
+++ 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 db@kavod.com.

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 [38048]
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:        ??? [1]
Responsible:           firefox [38048]
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.
Attached file test_case.html
Duplicate of this bug: 1288123
Valentin, could you look at this? The top of the stack at least is in a strcmp of some URL thing, I believe.
Flags: needinfo?(valentin.gosu)
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).
Summary: JS oneliner causes segfault on OS X → URL parsing causes crash
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
Flags: needinfo?(valentin.gosu)
Whiteboard: [necko-active]
thanks valentin
Attached patch Backout bug 1288482 (obsolete) — Splinter Review
MozReview-Commit-ID: J3LmMfQ854V
Attachment #8774136 - Flags: review?(mcmanus)
MozReview-Commit-ID: J3LmMfQ854V
Attachment #8774154 - Flags: review?(mcmanus)
Attachment #8774136 - Attachment is obsolete: true
Attachment #8774136 - 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.
Duplicate of this bug: 1288481
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+
https://hg.mozilla.org/mozilla-central/rev/405ec1364547
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
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+
Group: network-core-security → core-security-release
Blocks: 1042347
Flags: qe-verify-
Whiteboard: [necko-active] → [necko-active][post-critsmash-triage]
Whiteboard: [necko-active][post-critsmash-triage] → [necko-active][post-critsmash-triage][adv-main50+]
Alias: CVE-2016-5292
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.