Bug 1037641 (CVE-2014-1567)

Mozilla Firefox DirectionalityUtils Use-After-Free Remote Code Execution Vulnerability (ZDI-CAN-2394)

VERIFIED FIXED in Firefox 32

Status

()

defect
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: curtisk, Assigned: smontagu)

Tracking

({csectype-uaf, regression, sec-critical})

30 Branch
mozilla34
x86_64
Windows 8.1
Points:
---
Dependency tree / graph
Bug Flags:
sec-bounty +
in-testsuite ?

Firefox Tracking Flags

(firefox30 wontfix, firefox31+ wontfix, firefox32+ verified, firefox33+ verified, firefox34+ verified, firefox-esr2432+ verified, firefox-esr3132+ verified, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

(Whiteboard: [reporter-external][adv-main32+][adv-esr31.1+][adv-esr24.8+])

Attachments

(4 attachments, 3 obsolete attachments)

ZDI-CAN-2394: Mozilla Firefox DirectionalityUtils Use-After-Free Remote Code Execution Vulnerability


-- CVSS -----------------------------------------

6.8, AV:N/AC:M/Au:N/C:P/I:P/A:P


-- ABSTRACT -------------------------------------

HP's Zero Day Initiative has identified a vulnerability affecting the following products:

  Mozilla Firefox


-- VULNERABILITY DETAILS ------------------------

Verified against Firefox 30.0 Stable on Windows 8.1 as well as the latest nightly as of July 1st.

```
(f48.9c4): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=095712b0 ebx=0c0c0000 ecx=0ddc2600 edx=0ddc2600 esi=0ddc2600 edi=0c0c0000
eip=095712b0 esp=00b1dcd8 ebp=00b1dd04 iopl=0         nv up ei ng nz na pe cy
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00010287
095712b0 27              daa
0:000> kv
ChildEBP RetAddr  Args to Child              
WARNING: Frame IP not in any known module. Following frames may be wrong.
00b1dcd4 60526d2c 00b1dcf0 00000002 0ddc2600 0x95712b0
00b1dd04 60cfa9f3 00000001 0abb15b0 08712801 xul!mozilla::dom::Element::UpdateState+0x16c (FPO: [Non-Fpo]) (CONV: thiscall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\content\base\src\element.cpp @ 182]
00b1dd18 61181bbc 00000002 60e78875 0abb15b0 xul!mozilla::dom::Element::SetDirectionality+0x79a803 (CONV: thiscall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\obj-firefox\dist\include\mozilla\dom\element.h @ 325]
00b1dd20 60e78875 0abb15b0 00b1dd50 00000002 xul!mozilla::nsTextNodeDirectionalityMap::SetNodeDirection+0x14 (FPO: [2,0,0]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\content\base\src\directionalityutils.cpp @ 494]
00b1dd34 60ed1d0f 0abb15b0 61181ba8 00b1dd50 xul!nsCheapSet<nsPtrHashKey<mozilla::dom::Element> >::EnumerateEntries+0x2b (FPO: [Non-Fpo]) (CONV: thiscall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\obj-firefox\dist\include\nscheapsets.h @ 68]
00b1dd44 60fe2c7c 0abb15b0 00000002 60d24b09 xul!mozilla::nsTextNodeDirectionalityMap::UpdateAutoDirection+0x13 (FPO: [2,0,0]) (CONV: thiscall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\content\base\src\directionalityutils.cpp @ 530]
00b1dd50 60d24b09 08712840 00000002 00b1df1c xul!mozilla::nsTextNodeDirectionalityMap::UpdateTextNodeDirection+0x14 (FPO: [2,0,0]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\content\base\src\directionalityutils.cpp @ 571]
00b1deb4 611b700b 08712840 00000002 102b9f58 xul!nsGenericDOMDataNode::SetTextInternal+0x853d19 (CONV: thiscall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\content\base\src\nsgenericdomdatanode.cpp @ 320]
00b1decc 611b7133 00000000 00000002 00b1df1c xul!nsGenericDOMDataNode::ReplaceData+0x1d (FPO: [3,0,4]) (CONV: thiscall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\content\base\src\nsgenericdomdatanode.cpp @ 267]
00b1dee4 611b72c2 08712840 00000000 00000002 xul!nsGenericDOMDataNode::ReplaceData+0x15 (FPO: [Non-Fpo]) (CONV: thiscall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\content\base\src\nsgenericdomdatanode.h @ 220]
00b1df38 6064c573 0c1e2e40 00b1df74 08712840 xul!mozilla::dom::CharacterDataBinding::replaceData+0xef (FPO: [Non-Fpo]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\obj-firefox\dom\bindings\characterdatabinding.cpp @ 281]
00b1df74 636f88f0 0c1e2e40 00000003 00000028 xul!mozilla::dom::GenericBindingMethod+0xc3 (FPO: [3,6,0]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\dom\bindings\bindingutils.cpp @ 2278]
00b1e270 636f9c16 0c1e2e40 00000000 00b1e2e0 mozjs!js::Invoke+0x2d0 (FPO: [Non-Fpo]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\js\src\vm\interpreter.cpp @ 476]
00b1e320 636d483b 0c1e2e40 00b1e398 00b1e388 mozjs!js::Invoke+0x216 (FPO: [Non-Fpo]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\js\src\vm\interpreter.cpp @ 532]
00b1e3c4 00b1e43c 2580513d 0c1e2e40 00b1e48c mozjs!js::jit::DoCallFallback+0x1eb (FPO: [Non-Fpo]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\js\src\jit\baselineic.cpp @ 8138]
00b1e3e0 00b1e3ec 00b1e3ec 00000000 ffffff82 0xb1e43c
00b1e4e8 636e0879 2c1230d4 00000001 00b1f4e0 0xb1e3ec
00b1e550 6377a18b 0c1e2e40 00b1f188 00000003 mozjs!EnterBaseline+0xe9 (FPO: [Non-Fpo]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\js\src\jit\baselinejit.cpp @ 123]
00b1e5b4 636f488e 0f02c62a 0c1e2e40 08c8e800 mozjs!js::jit::EnterBaselineAtBranch+0x17b (FPO: [1,22,0]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\js\src\jit\baselinejit.cpp @ 207]
00b1f188 636f9937 00000000 10513920 00b1f5f0 mozjs!Interpret+0x5a5e (FPO: [Non-Fpo]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\js\src\vm\interpreter.cpp @ 1714]
00b1f478 636f9c16 0c1e2e40 00000000 00b1f4e8 mozjs!js::Invoke+0x1317 (FPO: [Non-Fpo]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\js\src\vm\interpreter.cpp @ 495]
00b1f52c 6374fa71 0c1e2e40 00b1f680 00b1f5c0 mozjs!js::Invoke+0x216 (FPO: [Non-Fpo]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\js\src\vm\interpreter.cpp @ 532]
00b1f554 60431727 0c1e2e40 00b1f680 00b1f5c0 mozjs!JS::Call+0x21 (FPO: [Non-Fpo]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\js\src\jsapi.cpp @ 4916]
00b1f640 60449ff2 0355d4a0 00b1f680 0adb1b98 xul!mozilla::dom::Function::Call+0x107 (FPO: [4,50,4]) (CONV: thiscall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\obj-firefox\dom\bindings\functionbinding.cpp @ 35]
00b1f744 605836db 00b1f780 0adb1b98 00b1f798 xul!mozilla::dom::Function::Call<nsCOMPtr<nsISupports> >+0xc2 (FPO: [3,56,0]) (CONV: thiscall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\obj-firefox\dist\include\mozilla\dom\functionbinding.h @ 58]
00b1f84c 6057fc91 0c578080 0d42ac40 00b1f8dc xul!nsGlobalWindow::RunTimeoutHandler+0x11b (FPO: [1,60,0]) (CONV: thiscall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\dom\base\nsglobalwindow.cpp @ 11877]
00b1f8bc 6042c630 0d42ac40 08d4a200 6042c5ec xul!nsGlobalWindow::RunTimeout+0x2d1 (FPO: [1,23,4]) (CONV: thiscall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\dom\base\nsglobalwindow.cpp @ 12102]
00b1f8dc 6056ba67 08d4a200 0d42ac40 0351b240 xul!nsGlobalWindow::TimerCallback+0x44 (FPO: [Non-Fpo]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\dom\base\nsglobalwindow.cpp @ 12347]
00b1f8f8 6056bbe4 604e6207 0a18f1f0 00b1f9b8 xul!nsTimerImpl::Fire+0xc7 (FPO: [0,1,4]) (CONV: thiscall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\xpcom\threads\nstimerimpl.cpp @ 551]
00b1f8fc 604e6207 0a18f1f0 00b1f9b8 0350e9b0 xul!nsTimerEvent::Run+0x14 (FPO: [1,0,0]) (CONV: stdcall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\xpcom\threads\nstimerimpl.cpp @ 637]
00b1f97c 604c8b1d 0351b201 00000001 00b1f994 xul!nsThread::ProcessNextEvent+0x3b7 (FPO: [3,26,0]) (CONV: stdcall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\xpcom\threads\nsthread.cpp @ 694]
00b1f98c 606c5e1d 0351b201 00000001 0352e360 xul!NS_ProcessNextEvent+0x2d (FPO: [2,0,0]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\xpcom\glue\nsthreadutils.cpp @ 263]
00b1f9b8 606b9440 0052e360 bcab8a31 0351b240 xul!mozilla::ipc::MessagePump::Run+0x9b (FPO: [Non-Fpo]) (CONV: thiscall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\ipc\glue\messagepump.cpp @ 136]
00b1f9f0 606b94cf 0351bbe0 00000001 00b1fa00 xul!MessageLoop::RunHandler+0x51 (FPO: [Non-Fpo]) (CONV: thiscall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\ipc\chromium\src\base\message_loop.cc @ 220]
00b1fa10 60618a5e 80000000 088315c0 6061fbaf xul!MessageLoop::Run+0x19 (FPO: [Non-Fpo]) (CONV: thiscall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\ipc\chromium\src\base\message_loop.cc @ 194]
00b1fa1c 6061fbaf 0351bbe0 60be25cf 0351bbe0 xul!nsBaseAppShell::Run+0x2c (FPO: [1,0,0]) (CONV: stdcall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\widget\xpwidgets\nsbaseappshell.cpp @ 166]
00b1fa30 605a9c2a 088315c0 721c1306 00b1fb44 xul!nsAppShell::Run+0x19 (FPO: [1,0,4]) (CONV: stdcall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\widget\windows\nsappshell.cpp @ 165]
00b1fb04 606072c6 00b1fb44 00b1fc6c 00b1fb44 xul!XREMain::XRE_mainRun+0x453 (FPO: [Non-Fpo]) (CONV: thiscall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\toolkit\xre\nsapprunner.cpp @ 4010]
00b1fb24 60620d14 00b1fb44 00000000 024f2ff8 xul!XREMain::XRE_main+0xe8 (FPO: [Non-Fpo]) (CONV: thiscall) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\toolkit\xre\nsapprunner.cpp @ 4079]
00b1fc3c 013616dd 00000001 024f2ff8 00b1fc6c xul!XRE_main+0x30 (FPO: [Non-Fpo]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\toolkit\xre\nsapprunner.cpp @ 4291]
00b1fdd0 013619a2 00000001 024f2ff8 0351e0a0 firefox!do_main+0x283 (FPO: [Non-Fpo]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\browser\app\nsbrowserapp.cpp @ 282]
00b1fe64 01361aad 00000001 024f2ff8 00000000 firefox!NS_internal_main+0x11d (FPO: [Non-Fpo]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\browser\app\nsbrowserapp.cpp @ 643]
00b1fe98 0136237b 00000000 00f63f98 00f67f70 firefox!wmain+0xf0 (FPO: [Non-Fpo]) (CONV: cdecl) [c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\toolkit\xre\nswindowswmain.cpp @ 112]
00b1fedc 75ec17ad 7fccd000 00b1ff2c 7779db0e firefox!__tmainCRTStartup+0x122 (FPO: [Non-Fpo]) (CONV: cdecl) [f:\dd\vctools\crt_bld\self_x86\crt\src\crtexe.c @ 552]
00b1fee8 7779db0e 7fccd000 b83c25bc 00000000 KERNEL32!BaseThreadInitThunk+0xe (FPO: [Non-Fpo])
00b1ff2c 7779dae7 ffffffff 777e4b0e 00000000 ntdll!__RtlUserThreadStart+0x20 (FPO: [Non-Fpo])
00b1ff3c 00000000 0136249c 7fccd000 00000000 ntdll!_RtlUserThreadStart+0x1b (FPO: [Non-Fpo])
0:000> !lmi mozjs
Loaded Module Info: [mozjs] 
         Module: mozjs
   Base Address: 63650000
     Image Name: C:\Program Files\Mozilla Firefox\mozjs.dll
   Machine Type: 332 (I386)
     Time Stamp: 53912a18 Thu Jun 05 19:40:24 2014
           Size: 3b6000
       CheckSum: 3ad3a7
Characteristics: 2122  
Debug Data Dirs: Type  Size     VA  Pointer
             CODEVIEW    6f, 370168,  36f368 RSDS - GUID: {D312FB30-1992-4BDF-8364-B3FB1E13E7ED}
               Age: 2, Pdb: c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\obj-firefox\js\src\mozjs.pdb
     Image Type: FILE     - Image read successfully from debugger.
                 C:\Program Files\Mozilla Firefox\mozjs.dll
    Symbol Type: PDB      - Symbols loaded successfully from symbol server.
                 z:\export\symbols\mozjs.pdb\D312FB3019924BDF8364B3FB1E13E7ED2\mozjs.pdb
       Compiler: Linker - front end [0.0 bld 0] - back end [10.0 bld 30319]
    Load Report: private symbols & lines, source indexed 
                 z:\export\symbols\mozjs.pdb\D312FB3019924BDF8364B3FB1E13E7ED2\mozjs.pdb
0:000> lmvm mozjs
start    end        module name
63650000 63a06000   mozjs      (private pdb symbols)  z:\export\symbols\mozjs.pdb\D312FB3019924BDF8364B3FB1E13E7ED2\mozjs.pdb
    Loaded symbol image file: C:\Program Files\Mozilla Firefox\mozjs.dll
    Image path: C:\Program Files\Mozilla Firefox\mozjs.dll
    Image name: mozjs.dll
    Timestamp:        Thu Jun 05 19:40:24 2014 (53912A18)
    CheckSum:         003AD3A7
    ImageSize:        003B6000
    Translations:     0000.04b0 0000.04e4 0409.04b0 0409.04e4
0:000> !lmi xul
Loaded Module Info: [xul] 
         Module: xul
   Base Address: 60380000
     Image Name: C:\Program Files\Mozilla Firefox\xul.dll
   Machine Type: 332 (I386)
     Time Stamp: 539141b1 Thu Jun 05 21:21:05 2014
           Size: 1826000
       CheckSum: 16e4463
Characteristics: 2122  
Debug Data Dirs: Type  Size     VA  Pointer
             CODEVIEW    76, 14afff8, 14af3f8 RSDS - GUID: {0B4EEB73-D3AE-4584-B127-6A3931531AD3}
               Age: 2, Pdb: c:\builds\moz2_slave\rel-m-rel-w32_bld-000000000000\build\obj-firefox\toolkit\library\xul.pdb
     Image Type: FILE     - Image read successfully from debugger.
                 C:\Program Files\Mozilla Firefox\xul.dll
    Symbol Type: PDB      - Symbols loaded successfully from symbol server.
                 z:\export\symbols\xul.pdb\0B4EEB73D3AE4584B1276A3931531AD32\xul.pdb
       Compiler: Linker - front end [0.0 bld 0] - back end [10.0 bld 30319]
    Load Report: private symbols & lines, source indexed 
                 z:\export\symbols\xul.pdb\0B4EEB73D3AE4584B1276A3931531AD32\xul.pdb
0:000> lmvm xul
start    end        module name
60380000 61ba6000   xul        (private pdb symbols)  z:\export\symbols\xul.pdb\0B4EEB73D3AE4584B1276A3931531AD32\xul.pdb
    Loaded symbol image file: C:\Program Files\Mozilla Firefox\xul.dll
    Image path: C:\Program Files\Mozilla Firefox\xul.dll
    Image name: xul.dll
    Timestamp:        Thu Jun 05 21:21:05 2014 (539141B1)
    CheckSum:         016E4463
    ImageSize:        01826000
    File version:     30.0.0.5269
    Product version:  30.0.0.5269
    File flags:       0 (Mask 3F)
    File OS:          4 Unknown Win32
    File type:        2.0 Dll
    File date:        00000000.00000000
    Translations:     0000.04b0
    CompanyName:      Mozilla Foundation
    ProductName:      Firefox
    InternalName:     Firefox
    OriginalFilename: Firefox
    ProductVersion:   30.0
    FileVersion:      30.0
    FileDescription:  30.0
    LegalCopyright:   License: MPL 2
    LegalTrademarks:  Mozilla
    Comments:         Mozilla


When the spray lands properly:
(94.f60): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=c1c2c3c4 ebx=0c0c0000 ecx=0b7fdce0 edx=0b7fdce0 esi=0b7fdce0 edi=0c0c0000
eip=c1c2c3c4 esp=00f9dcd8 ebp=00f9dd04 iopl=0         ov up ei pl nz na pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00010a06
c1c2c3c4 ??              ???


-- CREDIT ---------------------------------------

This vulnerability was discovered by:

   regenrecht working with HP's Zero Day Initiative
Attachment #8454679 - Attachment description: bugbounty.data → zdi-disclosures@tippingpoint.com,,2014-07-11,,,false,"regenrecht working with HP's Zero Day Initiative"
Flags: sec-bounty?
Whiteboard: [reporter-external]
OS: All → Windows 8.1
Hardware: All → x86_64
Version: 31 Branch → 30 Branch
Definitely repros on Windows 7. Had to reload three or four times (got the "failed" alert).
bp-4b2ea53e-eaf7-4452-b366-fe9c62140711  (EXCEPTION_STACK_BUFFER_OVERRUN)

I also saw EXCEPTION_ACCESS_VIOLATION_READ which can be a symptom of UAF.
Assignee: nobody → smontagu
Group: layout-core-security
Component: Security → Layout: Text
Tracking for 31. FYI, 31 RC/beta10 go to build is supposed to be today.
Assignee

Comment 5

5 years ago
This is quite interesting, and we should check out whether there are other places where a similar attack could be devised: the testcase manages to get the TextDirectionalityMap into an inconsistent state by first changing a single low surrogate (line 32 in view-source:https://bug1037641.bugzilla.mozilla.org/attachment.cgi?id=8454676&t=AvJXXXTK6d) and then changing back the whole surrogate pair (line 58)
Assignee

Comment 6

5 years ago
Posted patch Suggested patch (obsolete) — Splinter Review
This addresses two issues (with the assumption that we will go on allowing to modify text with a single surrogate code point):

the value returned by GetDirectionFromText in DirectionalityUtils didn't take into account surrogate pairs.

SetDirectionFromChangedTextNode looked at the new text in isolation to get its bidi directionality, which in the case of the single surrogate code point prevented us from getting the correct directionality value. The patch splits SetDirectionFromChangedTextNode into two and gets the directionality from the whole text node before and after the change. Note that this could have performance impact e.g. when appending text to a long text node, but I don't think that the impact will be huge in normal scenarii.
Attachment #8455623 - Flags: review?(ehsan)

Updated

5 years ago
Attachment #8455623 - Flags: review?(ehsan) → review+
Assignee

Comment 8

5 years ago
Posted patch Patch v.2Splinter Review
Apologies for the churn. I realized on looking over the patch that this change:

-SetDirectionFromChangedTextNode(nsIContent* aTextNode, uint32_t aOffset,
-                                const char16_t* aBuffer, uint32_t aLength,
-                                bool aNotify)
+TextNodeWillChangeDirection(nsIContent* aTextNode, Directionality* aOldDir,
+                            uint32_t* aFirstStrong)
 {
   if (!NodeAffectsDirAutoAncestor(aTextNode)) {
     nsTextNodeDirectionalityMap::EnsureMapIsClearFor(aTextNode);
+    *aFirstStrong = UINT32_MAX;
     return;
   }

... is exactly backwards: I should have written |*aFirstStrong = 0|, but I think it's better just to return a boolean and not use a magic value for aFirstStrong.
Attachment #8455623 - Attachment is obsolete: true
Attachment #8456058 - Flags: review?(ehsan)
Assignee

Comment 9

5 years ago
Comment on attachment 8456058 [details] [diff] [review]
Patch v.2

[Security approval request comment]
How easily could an exploit be constructed based on the patch? I think it's possible: the change at https://bugzilla.mozilla.org/attachment.cgi?id=8456058&action=diff#a/content/base/src/DirectionalityUtils.cpp_sec2 is a huge clue that surrogate pairs are involved, and getting from there to an exploit would certainly be no harder than creating the attached exploit in the first place.

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? All supported branches

If not all supported branches, which bug introduced the flaw? Bug 548206

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Creating backports will be trivial.

How likely is this patch to cause regressions; how much testing does it need? We have a lot of automatic tests for modifying text nodes with dir=auto already, but adding some more can always help
Attachment #8456058 - Flags: sec-approval?

Comment 10

5 years ago
Comment on attachment 8456058 [details] [diff] [review]
Patch v.2

Review of attachment 8456058 [details] [diff] [review]:
-----------------------------------------------------------------

Ah, I should have spotted that...
Attachment #8456058 - Flags: review?(ehsan) → review+
This is going to be delayed for checkin for a few weeks because we just made the Firefox 31 builds for release on this coming tuesday.
Whiteboard: [reporter-external] → [reporter-external][checkin on 8/5]
Comment on attachment 8456058 [details] [diff] [review]
Patch v.2

sec-approval+ for checkin on trunk after 8/5
Attachment #8456058 - Flags: sec-approval? → sec-approval+
Flags: sec-bounty? → sec-bounty+
This is okay to check in now.
Flags: needinfo?(smontagu)
Whiteboard: [reporter-external][checkin on 8/5] → [reporter-external]
https://hg.mozilla.org/mozilla-central/rev/457ca2777ef4
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Please request Aurora/Beta/esr31/esr24 approval on this when you get a chance.
Flags: needinfo?(smontagu)
Approval ping? :)
Comment on attachment 8456058 [details] [diff] [review]
Patch v.2

Approval Request Comment
[Feature/regressing bug #]: Bug 548206
[User impact if declined]: sec-crit
[Describe test coverage new/current, TBPL]: On trunk for 2 weeks with no known regressions. Applies cleanly to 30+. Trivial rebasing was needed for <30, but was green on Try.
[Risks and why]: Per comment 9, this is well-tested code and no regressions have turned up since it landed on trunk.
[String/UUID change made/needed]: none
Attachment #8456058 - Flags: approval-mozilla-esr31?
Attachment #8456058 - Flags: approval-mozilla-esr24?
Attachment #8456058 - Flags: approval-mozilla-beta?
Attachment #8456058 - Flags: approval-mozilla-aurora?
Flags: needinfo?(smontagu)
Attachment #8456058 - Flags: approval-mozilla-esr31?
Attachment #8456058 - Flags: approval-mozilla-esr31+
Attachment #8456058 - Flags: approval-mozilla-esr24?
Attachment #8456058 - Flags: approval-mozilla-esr24+
Attachment #8456058 - Flags: approval-mozilla-beta?
Attachment #8456058 - Flags: approval-mozilla-beta+
Attachment #8456058 - Flags: approval-mozilla-aurora?
Attachment #8456058 - Flags: approval-mozilla-aurora+
Whiteboard: [reporter-external] → [reporter-external][adv-main32+][adv-esr31.1+][adv-esr24.8+]
Confirmed crash on Fx34, 2014-08-04.
Verified fixed in Fx32, Fx33 and Fx34, 2014-08-22.
Verified fixed in Fx24.8.0esr, release.
Verified fixed in Fx31.1.0esr, release.
Assigning CVE CVE-2014-1567 to this issue.
Alias: ZDI-CAN-2394 → CVE-2014-1567
Summary: Mozilla Firefox DirectionalityUtils Use-After-Free Remote Code Execution Vulnerability → Mozilla Firefox DirectionalityUtils Use-After-Free Remote Code Execution Vulnerability (ZDI-CAN-2394)
Group: layout-core-security
Attachment #8454679 - Attachment description: zdi-disclosures@tippingpoint.com,,2014-07-11,,,false,"regenrecht working with HP's Zero Day Initiative" → removed data
Attachment #8454679 - Attachment filename: bugbounty.data → bugbounty.data1
Attachment #8454679 - Attachment is private: false

Comment 23

5 years ago
Comment on attachment 8455659 [details] [diff] [review]
Testcase for checkin after the bug is open

Review of attachment 8455659 [details] [diff] [review]:
-----------------------------------------------------------------

Please replace this slow and unreliable "gc" method with a call to SpecialPowers.gc().
Assignee

Comment 24

5 years ago
Like this? SpecialPowers.gc() doesn't seem to be documented much to speak of, so I'm not really sure what I'm doing. I also haven't tested yet that this version fails if I revert the patch.
Attachment #8455659 - Attachment is obsolete: true
Attachment #8528490 - Flags: feedback?(jruderman)
Assignee

Comment 25

5 years ago
With the right patch this time
Attachment #8528490 - Attachment is obsolete: true
Attachment #8528490 - Flags: feedback?(jruderman)
Attachment #8528491 - Flags: feedback?(jruderman)

Comment 26

5 years ago
Comment on attachment 8528491 [details] [diff] [review]
Testcase v.2 (for checkin after the bug is open)

Review of attachment 8528491 [details] [diff] [review]:
-----------------------------------------------------------------

This might be the first crashtest to use SpecialPowers, even though bug 792029 landed two years ago. Weird!

I'm confused about how this test uses setTimeout. Usually a crashtest that uses setTimeout will use <html class="reftest-wait"> and end the setTimeout chain with document.documentElement.removeAttribute("class"). But instead this one seems to end with alert("failed.")?
Attachment #8528491 - Flags: feedback?(jruderman)

Updated

4 years ago
Group: core-security → core-security-release
Blocks: DirAuto
Keywords: regression
Group: core-security-release
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.