831673 - UAF with SVG.createMatrix with ASAN

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Nils]

Attachment: testcase (crashes firefox)

The attached testcase crashes the ASAN build of latest nightly. Seen crashes in non-ASAN builds as well. The testcase require Jesse's quitter extension for garbage collection (https://www.squarefree.com/extensions/quitter.xpi).

ASAN output:
==16048== ERROR: AddressSanitizer heap-use-after-free on address 0x7fef3c367a98 at pc 0x7fef6d259c88 bp 0x7fffd1382350 sp 0x7fffd1382348
READ of size 8 at 0x7fef3c367a98 thread T0
    #0 0x7fef6d259c87 in _ZN27nsCycleCollectingAutoRefCnt4incrEPv /builds/slave/try-lnx64/build/../../../../dist/include/nsISupportsImpl.h:132
    #1 0x7fef6e652b3a in _ZN7mozilla3dom19SVGTransformBindingL13genericGetterEP9JSContextjPN2JS5ValueE /builds/slave/try-lnx64/build/obj-firefox/dom/bindings/SVGTransformBinding.cpp:402
    #2 0x7fef6fc55bf7 in _ZNK10JSFunction6nativeEv /builds/slave/try-lnx64/build/js/src/jscntxtinlines.h:378
    #3 0x7fef6fc56b56 in _ZN2js6InvokeEP9JSContextRNS_15InvokeArgsGuardENS_14MaybeConstructE /builds/slave/try-lnx64/build/js/src/jsinterp.h:112
    #4 0x7fef6fc57b33 in _ZN2js20InvokeGetterOrSetterEP9JSContextP8JSObjectRKN2JS5ValueEjPS5_S8_ /builds/slave/try-lnx64/build/js/src/jsinterp.cpp:512
    #5 0x7fef6fccbfde in _ZN2js5Shape3getEP9JSContextN2JS6HandleIP8JSObjectEES6_S6_NS3_13MutableHandleINS3_5ValueEEE /builds/slave/try-lnx64/build/js/src/jsscopeinlines.h:296
    #6 0x7fef6fcbc71b in _ZL18js_NativeGetInlineP9JSContextN2JS6HandleIP8JSObjectEES5_S5_NS2_IPN2js5ShapeEEEjNS1_13MutableHandleINS1_5ValueEEE /builds/slave/try-lnx64/build/js/src/jsobj.cpp:3408
    #7 0x7fef6fc5d383 in _ZN2js20GetPropertyOperationEP9JSContextP8JSScriptPhN2JS13MutableHandleINS5_5ValueEEES8_ /builds/slave/try-lnx64/build/js/src/jsinterpinlines.h:290
    #8 0x7fef6fc395f0 in _ZN2js9InterpretEP9JSContextPNS_10StackFrameENS_10InterpModeE /builds/slave/try-lnx64/build/js/src/jsinterp.cpp:2235
    #9 0x7fef6fc31706 in _ZN2js9RunScriptEP9JSContextN2JS6HandleIP8JSScriptEEPNS_10StackFrameE /builds/slave/try-lnx64/build/js/src/jsinterp.cpp:348
    #10 0x7fef6fc55af2 in _ZN2js12InvokeKernelEP9JSContextN2JS8CallArgsENS_14MaybeConstructE /builds/slave/try-lnx64/build/js/src/jsinterp.cpp:406
    #11 0x7fef6fc56b56 in _ZN2js6InvokeEP9JSContextRNS_15InvokeArgsGuardENS_14MaybeConstructE /builds/slave/try-lnx64/build/js/src/jsinterp.h:112
    #12 0x7fef6faf1107 in _Z20JS_CallFunctionValueP9JSContextP8JSObjectN2JS5ValueEjPS4_S5_ /builds/slave/try-lnx64/build/js/src/jsapi.cpp:5831
    #13 0x7fef6e438cd0 in _ZN7mozilla3dom19EventHandlerNonNull4CallEP9JSContextP8JSObjectP11nsIDOMEventRNS_11ErrorResultE /builds/slave/try-lnx64/build/obj-firefox/dom/bindings/EventHandlerBinding.cpp:47
    #14 0x7fef6c8b8110 in _ZN7mozilla3dom19EventHandlerNonNull4CallIP11nsISupportsEEN2JS5ValueERKT_P11nsIDOMEventRNS_11ErrorResultE /builds/slave/try-lnx64/build/../../../dist/include/mozilla/dom/EventHandlerBinding.h:61
    #15 0x7fef6c06be58 in _ZNK8nsRefPtrI19nsIDOMEventListenerEcvPS0_Ev /builds/slave/try-lnx64/build/content/events/src/nsEventListenerManager.cpp:922
    #16 0x7fef6c0d331c in _ZN22nsEventTargetChainItem13CurrentTargetEv /builds/slave/try-lnx64/build/content/events/src/nsEventListenerManager.h:278
0x7fef3c367a98 is located 24 bytes inside of 88-byte region [0x7fef3c367a80,0x7fef3c367ad8)
freed by thread T0 here:
    #0 0x4359e0 in free ??:0
    #1 0x7fef6d259ef4 in _ZN27nsCycleCollectingAutoRefCnt20stabilizeForDeletionEv /builds/slave/try-lnx64/build/../../../../dist/include/mozilla/mozalloc.h:224
previously allocated by thread T0 here:
    #0 0x435aa0 in __interceptor_malloc ??:0
    #1 0x7fef72c41288 in moz_xmalloc /builds/slave/try-lnx64/build/memory/mozalloc/mozalloc.cpp:54
    #2 0x7fef6e652b3a in _ZN7mozilla3dom19SVGTransformBindingL13genericGetterEP9JSContextjPN2JS5ValueE /builds/slave/try-lnx64/build/obj-firefox/dom/bindings/SVGTransformBinding.cpp:402
    #3 0x7fef6fc55bf7 in _ZNK10JSFunction6nativeEv /builds/slave/try-lnx64/build/js/src/jscntxtinlines.h:378
    #4 0x7fef6fc56b56 in _ZN2js6InvokeEP9JSContextRNS_15InvokeArgsGuardENS_14MaybeConstructE /builds/slave/try-lnx64/build/js/src/jsinterp.h:112
    #5 0x7fef6fc57b33 in _ZN2js20InvokeGetterOrSetterEP9JSContextP8JSObjectRKN2JS5ValueEjPS5_S8_ /builds/slave/try-lnx64/build/js/src/jsinterp.cpp:512
    #6 0x7fef6fccbfde in _ZN2js5Shape3getEP9JSContextN2JS6HandleIP8JSObjectEES6_S6_NS3_13MutableHandleINS3_5ValueEEE /builds/slave/try-lnx64/build/js/src/jsscopeinlines.h:296
    #7 0x7fef6fcbc71b in _ZL18js_NativeGetInlineP9JSContextN2JS6HandleIP8JSObjectEES5_S5_NS2_IPN2js5ShapeEEEjNS1_13MutableHandleINS1_5ValueEEE /builds/slave/try-lnx64/build/js/src/jsobj.cpp:3408
    #8 0x7fef6fc5d383 in _ZN2js20GetPropertyOperationEP9JSContextP8JSScriptPhN2JS13MutableHandleINS5_5ValueEEES8_ /builds/slave/try-lnx64/build/js/src/jsinterpinlines.h:290
    #9 0x7fef6fc395f0 in _ZN2js9InterpretEP9JSContextPNS_10StackFrameENS_10InterpModeE /builds/slave/try-lnx64/build/js/src/jsinterp.cpp:2235
    #10 0x7fef6fc31706 in _ZN2js9RunScriptEP9JSContextN2JS6HandleIP8JSScriptEEPNS_10StackFrameE /builds/slave/try-lnx64/build/js/src/jsinterp.cpp:348
    #11 0x7fef6fc55af2 in _ZN2js12InvokeKernelEP9JSContextN2JS8CallArgsENS_14MaybeConstructE /builds/slave/try-lnx64/build/js/src/jsinterp.cpp:406
    #12 0x7fef6fc56b56 in _ZN2js6InvokeEP9JSContextRNS_15InvokeArgsGuardENS_14MaybeConstructE /builds/slave/try-lnx64/build/js/src/jsinterp.h:112
    #13 0x7fef6faf1107 in _Z20JS_CallFunctionValueP9JSContextP8JSObjectN2JS5ValueEjPS4_S5_ /builds/slave/try-lnx64/build/js/src/jsapi.cpp:5831
    #14 0x7fef6e438cd0 in _ZN7mozilla3dom19EventHandlerNonNull4CallEP9JSContextP8JSObjectP11nsIDOMEventRNS_11ErrorResultE /builds/slave/try-lnx64/build/obj-firefox/dom/bindings/EventHandlerBinding.cpp:47
    #15 0x7fef6c8b8110 in _ZN7mozilla3dom19EventHandlerNonNull4CallIP11nsISupportsEEN2JS5ValueERKT_P11nsIDOMEventRNS_11ErrorResultE /builds/slave/try-lnx64/build/../../../dist/include/mozilla/dom/EventHandlerBinding.h:61
    #16 0x7fef6c06be58 in _ZNK8nsRefPtrI19nsIDOMEventListenerEcvPS0_Ev /builds/slave/try-lnx64/build/content/events/src/nsEventListenerManager.cpp:922
    #17 0x7fef6c0d331c in _ZN22nsEventTargetChainItem13CurrentTargetEv /builds/slave/try-lnx64/build/content/events/src/nsEventListenerManager.h:278
Shadow byte and word:
  0x1ffde786cf53: fd
  0x1ffde786cf50: fd fd fd fd fd fd fd fd
More shadow bytes:
  0x1ffde786cf30: 00 00 00 00 00 00 00 00
  0x1ffde786cf38: 00 00 00 00 00 00 00 00
  0x1ffde786cf40: fa fa fa fa fa fa fa fa
  0x1ffde786cf48: fa fa fa fa fa fa fa fa
=>0x1ffde786cf50: fd fd fd fd fd fd fd fd
  0x1ffde786cf58: fd fd fd fd fd fd fd fd
  0x1ffde786cf60: fa fa fa fa fa fa fa fa
  0x1ffde786cf68: fa fa fa fa fa fa fa fa
  0x1ffde786cf70: 00 00 00 00 00 00 00 00
Stats: 405M malloced (464M for red zones) by 818803 calls
Stats: 47M realloced by 53857 calls
Stats: 372M freed by 657453 calls
Stats: 258M really freed by 331234 calls
Stats: 592M (151652 full pages) mmaped in 148 calls
  mmaps   by size class: 8:475107; 9:49146; 10:20475; 11:16376; 12:5120; 13:3072; 14:1280; 15:384; 16:1408; 17:1280; 18:48; 19:48; 20:36; 21:2;
  mallocs by size class: 8:674347; 9:72364; 10:26679; 11:27317; 12:6108; 13:4818; 14:2296; 15:577; 16:2279; 17:1835; 18:62; 19:68; 20:52; 21:1;
  frees   by size class: 8:539390; 9:56767; 10:21852; 11:23810; 12:4529; 13:4412; 14:2072; 15:468; 16:2162; 17:1820; 18:53; 19:66; 20:52;
  rfrees  by size class: 8:259841; 9:35794; 10:10446; 11:14617; 12:3021; 13:2483; 14:1674; 15:319; 16:1428; 17:1496; 18:38; 19:50; 20:27;
Stats: malloc large: 2019 small slow: 4443
==16048== ABORTING

Comment 1

[Robert Longson [:longsonr]]

David, is this down to your changes?

Comment 2

[David Zbarsky (:dzbarsky)]

I don't see a crash on this testcase in a trunk build with the quitter extension installed.  How do I reproduce it?

Comment 3

[Boris Zbarsky [:bzbarsky]]

So I don't understand how sSVGMatrixTearoffTable is supposed to work.  It doesn't hold a ref to the SVGMatrix.  So how does it make sure to not hold a stale pointer?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Looks like bug 817256 changed stuff here.  The changeset claims that we're now holding a strong ref, but of course we're doing nothing of the sort as far as I can tell.

Comment 5

[Nils]

The minimised testcase crashes only with ASAN for me. Latest ASAN build from 

https://people.mozilla.com/~choller/firefox/asan/20130117-mozilla-central-linux64-opt-712eca11a04e+asan.html

Comment 6

[David Zbarsky (:dzbarsky)]

https://mxr.mozilla.org/mozilla-central/source/content/svg/content/src/DOMSVGTransform.cpp#129 is broken.

I think we want to addref when adding to the table, and return non-addrefed SVGMatrix*

Comment 7

[David Zbarsky (:dzbarsky)]

Attachment: Patch

Comment 8

[David Zbarsky (:dzbarsky)]

Attachment: Patch

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 704101 [details] [diff] [review]
Patch

r=me

Comment 10

[David Zbarsky (:dzbarsky)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/abafe7350c6f

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/abafe7350c6f

Comment 12

[Al Billings [:abillings - ex-MoCo]]

What this bug Firefox 21 only?

Comment 14

[Al Billings [:abillings - ex-MoCo]]

The bounty for this bug is split between this bug and bug 831668, which were filed within 15 minutes of each other by external reports.