Use-after-free when creating dependent strings with an external base string

firefox53

firefox54

firefox55

People

(Reporter: arai, Assigned: jandem)

References

Details

(4 keywords, Whiteboard: [adv-main53+][adv-esr52.1+])

Attachments

(3 files)

Part 1 - Fix 7 years ago Jan de Mooij [:jandem] 6.58 KB, patch	h4writer : review+ Waldo : review+ lizzard : approval-mozilla-aurora+ lizzard : approval-mozilla-beta+ jcristau : approval-mozilla-esr52+ abillings : sec-approval+	Details \| Diff \| Splinter Review
Part 2 - Test, comments 7 years ago Jan de Mooij [:jandem] 6.56 KB, patch	Waldo : review+	Details \| Diff \| Splinter Review
Part 3 - Add debug check to CreateDependentString::generate 7 years ago Jan de Mooij [:jandem] 1.15 KB, patch	h4writer : review+	Details \| Diff \| Splinter Review

Reporter

Description

•

7 years ago

Steps to reproduce:
  1. download ASAN debug build from
     https://developer.mozilla.org/en-US/docs/Mozilla/Testing/Firefox_and_Address_Sanitizer
     https://index.taskcluster.net/v1/task/gecko.v2.mozilla-central.latest.firefox.linux64-asan-debug/artifacts/public/build/target.tar.bz2
  2. extract it
  3. run firefox
  4. in about:preferences, disable e10s and restart
  5. install UnMHT extension
     https://addons.mozilla.org/ja/firefox/addon/unmht/
     https://addons.mozilla.org/firefox/downloads/latest/unmht/addon-8051-latest.xpi
  6. open https://www.mozilla.org/en-US/security/advisories/mfsa2017-05/
  7. click UnMHT Toolbar button (orange one) to open menu, and click [Quick Save]

Actual Result:
  crash:

==1991==ERROR: AddressSanitizer: heap-use-after-free on address 0x611000773218 at pc 0x7f559dad0d8a bp 0x7ffc31c74350 sp 0x7ffc31c74348
READ of size 2 at 0x611000773218 thread T0
    #0 0x7f559dad0d89  (/home/osboxes/tmp/firefox-debug/libxul.so+0xbf62d89)
    #1 0x7f559daa2f6a  (/home/osboxes/tmp/firefox-debug/libxul.so+0xbf34f6a)
    #2 0x7f559daa2ebb  (/home/osboxes/tmp/firefox-debug/libxul.so+0xbf34ebb)
    #3 0x7f559dad47b6  (/home/osboxes/tmp/firefox-debug/libxul.so+0xbf667b6)
    #4 0x7f559daa33c2  (/home/osboxes/tmp/firefox-debug/libxul.so+0xbf353c2)
    #5 0x7f55950d8e0a  (/home/osboxes/tmp/firefox-debug/libxul.so+0x356ae0a)
    #6 0x7f55961ca070  (/home/osboxes/tmp/firefox-debug/libxul.so+0x465c070)
    #7 0x7f5598247126  (/home/osboxes/tmp/firefox-debug/libxul.so+0x66d9126)
    #8 0x7f55982cebf8  (/home/osboxes/tmp/firefox-debug/libxul.so+0x6760bf8)
    #9 0x7f559872a97d  (/home/osboxes/tmp/firefox-debug/libxul.so+0x6bbc97d)
    #10 0x7f559ccb84e9  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14a4e9)
    #11 0x7f559ccb7f70  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb149f70)
    #12 0x7f559ccb8e3e  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14ae3e)
    #13 0x7f559cca76a5  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb1396a5)
    #14 0x7f559cc9ba8a  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb12da8a)
    #15 0x7f559ccb80ac  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14a0ac)
    #16 0x7f559ccb8e3e  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14ae3e)
    #17 0x7f559ccb9061  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14b061)
    #18 0x7f559d578e7d  (/home/osboxes/tmp/firefox-debug/libxul.so+0xba0ae7d)
    #19 0x7f559ccb84e9  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14a4e9)
    #20 0x7f559ccb7f70  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb149f70)
    #21 0x7f559ccb8e3e  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14ae3e)
    #22 0x7f559ccb9061  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14b061)
    #23 0x7f559d7379b8  (/home/osboxes/tmp/firefox-debug/libxul.so+0xbbc99b8)
    #24 0x7f559d6fa0c1  (/home/osboxes/tmp/firefox-debug/libxul.so+0xbb8c0c1)
    #25 0x7f559d722df8  (/home/osboxes/tmp/firefox-debug/libxul.so+0xbbb4df8)
    #26 0x7f559d72523c  (/home/osboxes/tmp/firefox-debug/libxul.so+0xbbb723c)
    #27 0x7f559ccb84e9  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14a4e9)
    #28 0x7f559ccb823e  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14a23e)
    #29 0x7f559ccb8e3e  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14ae3e)
    #30 0x7f559cca76a5  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb1396a5)
    #31 0x7f559cc9ba8a  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb12da8a)
    #32 0x7f559ccb80ac  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14a0ac)
    #33 0x7f559ccb8e3e  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14ae3e)
    #34 0x7f559ccb9061  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14b061)
    #35 0x7f559d4d9f00  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb96bf00)
    #36 0x7f55961fc7a5  (/home/osboxes/tmp/firefox-debug/libxul.so+0x468e7a5)
    #37 0x7f5594fcd4ff  (/home/osboxes/tmp/firefox-debug/libxul.so+0x345f4ff)
    #38 0x7f5594fcc2c6  (/home/osboxes/tmp/firefox-debug/libxul.so+0x345e2c6)

0x611000773218 is located 24 bytes inside of 256-byte region [0x611000773200,0x611000773300)
freed by thread T0 here:
    #0 0x4b341b  (/home/osboxes/tmp/firefox-debug/firefox+0x4b341b)
    #1 0x7f559daa82f7  (/home/osboxes/tmp/firefox-debug/libxul.so+0xbf3a2f7)
    #2 0x7f559d4f0f1e  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb982f1e)
    #3 0x7f559618ddcf  (/home/osboxes/tmp/firefox-debug/libxul.so+0x461fdcf)
    #4 0x7f5596237667  (/home/osboxes/tmp/firefox-debug/libxul.so+0x46c9667)
    #5 0x7f5596235dcb  (/home/osboxes/tmp/firefox-debug/libxul.so+0x46c7dcb)
    #6 0x7f559620df48  (/home/osboxes/tmp/firefox-debug/libxul.so+0x469ff48)
    #7 0x7f559620da03  (/home/osboxes/tmp/firefox-debug/libxul.so+0x469fa03)
    #8 0x7f5596211a9c  (/home/osboxes/tmp/firefox-debug/libxul.so+0x46a3a9c)
    #9 0x7f559ccb84e9  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14a4e9)
    #10 0x7f559ccb7f70  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb149f70)
    #11 0x7f559ccb8e3e  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14ae3e)
    #12 0x7f559cca76a5  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb1396a5)
    #13 0x7f559cc9ba8a  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb12da8a)
    #14 0x7f559ccb80ac  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14a0ac)
    #15 0x7f559ccb8e3e  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14ae3e)
    #16 0x7f559ccb9061  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14b061)
    #17 0x7f559d7379b8  (/home/osboxes/tmp/firefox-debug/libxul.so+0xbbc99b8)
    #18 0x7f559d6fa0c1  (/home/osboxes/tmp/firefox-debug/libxul.so+0xbb8c0c1)
    #19 0x7f559d722df8  (/home/osboxes/tmp/firefox-debug/libxul.so+0xbbb4df8)
    #20 0x7f559d72523c  (/home/osboxes/tmp/firefox-debug/libxul.so+0xbbb723c)
    #21 0x7f559ccb84e9  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14a4e9)
    #22 0x7f559ccb823e  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14a23e)
    #23 0x7f559ccb8e3e  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14ae3e)
    #24 0x7f559ce91f04  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb323f04)
    #25 0x2b46c2c868ca  (<unknown module>)
    #26 0x621001d142e7  (<unknown module>)
    #27 0x2b46c2fd3300  (<unknown module>)
    #28 0x6210021a44a7  (<unknown module>)
    #29 0x2b46c2c7ae3a  (<unknown module>)

previously allocated by thread T0 here:
    #0 0x4b373b  (/home/osboxes/tmp/firefox-debug/firefox+0x4b373b)
    #1 0x7f5594e1a359  (/home/osboxes/tmp/firefox-debug/libxul.so+0x32ac359)
    #2 0x7f5594e14d1c  (/home/osboxes/tmp/firefox-debug/libxul.so+0x32a6d1c)
    #3 0x7f5594e1dd77  (/home/osboxes/tmp/firefox-debug/libxul.so+0x32afd77)
    #4 0x7f5594e09321  (/home/osboxes/tmp/firefox-debug/libxul.so+0x329b321)
    #5 0x7f5594e0ab21  (/home/osboxes/tmp/firefox-debug/libxul.so+0x329cb21)
    #6 0x7f5594e0886b  (/home/osboxes/tmp/firefox-debug/libxul.so+0x329a86b)
    #7 0x7f55970b4c63  (/home/osboxes/tmp/firefox-debug/libxul.so+0x5546c63)
    #8 0x7f55970b4eee  (/home/osboxes/tmp/firefox-debug/libxul.so+0x5546eee)
    #9 0x7f559828f452  (/home/osboxes/tmp/firefox-debug/libxul.so+0x6721452)
    #10 0x7f5598728c2a  (/home/osboxes/tmp/firefox-debug/libxul.so+0x6bbac2a)
    #11 0x7f559ccb84e9  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14a4e9)
    #12 0x7f559ccb7f70  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb149f70)
    #13 0x7f559ccb8e3e  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14ae3e)
    #14 0x7f559ccb9061  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb14b061)
    #15 0x7f559d4dae68  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb96ce68)
    #16 0x7f55960ea993  (/home/osboxes/tmp/firefox-debug/libxul.so+0x457c993)
    #17 0x7f55960f2555  (/home/osboxes/tmp/firefox-debug/libxul.so+0x4584555)
    #18 0x7f559611e6d6  (/home/osboxes/tmp/firefox-debug/libxul.so+0x45b06d6)
    #19 0x7f559d720d66  (/home/osboxes/tmp/firefox-debug/libxul.so+0xbbb2d66)
    #20 0x7f559cc472d2  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb0d92d2)
    #21 0x7f559d720ee4  (/home/osboxes/tmp/firefox-debug/libxul.so+0xbbb2ee4)
    #22 0x7f559cc472d2  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb0d92d2)
    #23 0x7f559d720ee4  (/home/osboxes/tmp/firefox-debug/libxul.so+0xbbb2ee4)
    #24 0x7f559cc472d2  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb0d92d2)
    #25 0x7f559cc47140  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb0d9140)
    #26 0x7f559ccc000c  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb15200c)
    #27 0x7f559ccd6ac5  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb168ac5)
    #28 0x7f559cca4f75  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb136f75)
    #29 0x7f559cc9ba8a  (/home/osboxes/tmp/firefox-debug/libxul.so+0xb12da8a)

SUMMARY: AddressSanitizer: heap-use-after-free (/home/osboxes/tmp/firefox-debug/libxul.so+0xbf62d89) 
Shadow bytes around the buggy address:
  0x0c22800e65f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c22800e6600: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c22800e6610: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c22800e6620: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c22800e6630: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
=>0x0c22800e6640: fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c22800e6650: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c22800e6660: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c22800e6670: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c22800e6680: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x0c22800e6690: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==1991==ABORTING


Without ASAN build, I confirm the hostname part of URL becomes random character (so I guess use-after-free happens here).
the extension doesn't contain any binary component, only JS and some XUL, so at least it won't result in UAF without a bug in firefox itself.
I confirmed the random character on OSX (64bit) and linux (64bit).

this looks like started happening around firefox 52.
I'll try bisecting shortly.

Reporter

Comment 1

•

7 years ago

regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=47828603cd9c74ac3192b92b5a3a9c59fece1a15&tochange=eed0b80462a2f573f22b990c003f5f42ebd3c1f1

Component: Untriaged → JavaScript Engine

Keywords: regression

Reporter

Updated

•

7 years ago

status-firefox52: --- → affected

status-firefox53: --- → affected

status-firefox54: --- → affected

status-firefox-esr52: --- → affected

Reporter

Comment 2

•

7 years ago

this is the string observed in the extension:

expected:
  https://www.mozilla.org/en-US/security/advisories/mfsa2017-05/

actual:
  https://\u204D\u6F6D\u5F7A\u6C70\u6361\u7365\u6820\u4A20\u494F\u204E\u6F6D\u5F7A\u6166\u6976\u6F63/en-US/security/advisories/mfsa2017-05/

Blocks: 1330593

Reporter

Comment 3

•

7 years ago

the string is `sParam.baseDir`, in _startSerializeDocument in res/modules/UnMHTSave.jsm
>         if (!sParam.hasBaseElement &&
>             sParam.baseDir &&
>             sParam.baseDir != "about:blank") {
>           let node = UnMHTSaveUtils.makeBaseNode(sParam.targetDocument,
>                                                  sParam.baseDir);
>           head.appendChild(node);
>           tmpNodes.push(node);
>         }

the value is calculated in _getBaseDir in res/modules/UnMHTSave.jsm
>   _getBaseDir: function(targetDocument, location, sParam) {
>     let hasBaseElement = false;
>     let baseDir
>       = arPathUtils.getBaseDir(UnMHTSaveUtils.getMHTLocation(location));
> ...
>     sParam.baseDir = baseDir;

I tried to check where the string appears first, but the issue disappears when observing the baseDir value in _getBaseDir...

Reporter

Comment 4

•

7 years ago

now looking for a related string passed to AutoStableStringChars.

what I expect:
  https://www.mozilla.org/en-US/security/advisories/mfsa2017-05/

what I confirmed in AutoStableStringChars::init so far:
  JSString* (0x1af0dc058) = char16_t * (0x1adf98708) = "https://www.mozilla.org/media/img/tabzilla/tabzilla-static.953a65a1f4a4.png"

I'll check other methods.

Reporter

Comment 5

•

7 years ago

I overlooked one more in AutoStableStringChars::init, but it looks like session restore data, and I don't think it could be reused by dependentstring or something...

JSString* (0x10d6d02f8) = char16_t * (0x12a780008) = "{"version":["sessionrestore",1],"windows":[{"tabs":[{"entries":[{"url":"https://www.mozilla.org/en-US/security/advisories/mfsa2017-05/","title":"Security vulnerabilities fixed in Firefox 52 \u2014 Mozilla","charset":"UTF-8","ID":1,"docshellUUID":"{e0826aff-5354-9d4c-b8c2-d6db3560d54c}","originalURI":"https://www.mozilla.org/en-US/security/advisories/mfsa2017-05/","triggeringPrincipal_base64":"SmIS26zLEdO3ZQBgsLbOywAAAAAAAAAAwAAAAAAAAEY=",....

Reporter

Comment 6

•

7 years ago

here's lines for backtrace, converted with addr2line

==680==ERROR: AddressSanitizer: heap-use-after-free on address 0x61100008de58 at pc 0x7f57e969bc63 bp 0x7ffea34eae90 sp 0x7ffea34eae88
READ of size 16 at 0x61100008de58 thread T0
    #0 /objdir-ff-asan/dist/include/mozilla/PodOperations.h:87
    #1 /js/src/vm/String.cpp:576
    #2 /js/src/vm/String.h:1359 (discriminator 1)
    #3 /objdir-ff-asan/dist/include/jsfriendapi.h:871
    #4 /objdir-ff-asan/dist/include/mozilla/dom/BindingUtils.h:1898
    #5 /dom/bindings/BindingUtils.cpp:2953 (discriminator 3)
    #6 /js/src/jscntxtinlines.h:282 (discriminator 3)
    #7 /js/src/vm/Interpreter.cpp:499
    #8 /js/src/vm/Interpreter.cpp:394
    #9 /js/src/vm/Interpreter.cpp:466 (discriminator 1)
    #10 /js/src/vm/Interpreter.cpp:512
    #11 /js/src/jsfun.cpp:1269 (discriminator 4)
    #12 /js/src/jscntxtinlines.h:282 (discriminator 3)
    #13 /js/src/vm/Interpreter.cpp:512
    #14 /js/src/proxy/Wrapper.cpp:165 (discriminator 3)
    #15 /js/src/proxy/CrossCompartmentWrapper.cpp:353 (discriminator 1)
    #16 /js/src/proxy/Proxy.cpp:464 (discriminator 1)
    #17 /js/src/proxy/Proxy.cpp:716 (discriminator 1)
    #18 /js/src/jscntxtinlines.h:282 (discriminator 3)
    #19 /js/src/vm/Interpreter.cpp:499
    #20 /js/src/vm/Interpreter.cpp:394
    #21 /js/src/vm/Interpreter.cpp:466 (discriminator 1)
    #22 /js/src/vm/Interpreter.cpp:512
    #23 /js/src/jsapi.cpp:2828 (discriminator 3)
    #24 /js/xpconnect/src/XPCWrappedJSClass.cpp:1214 (discriminator 4)
    #25 /xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp:120
    #26 /xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp:?

0x61100008de58 is located 24 bytes inside of 256-byte region [0x61100008de40,0x61100008df40)
freed by thread T0 here:
    #0 /llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:38
    #1 /js/src/vm/String-inl.h:414 (discriminator 2)
    #2 /js/xpconnect/src/XPCConvert.cpp:634 (discriminator 2)
    #3 /js/xpconnect/src/XPCWrappedNative.cpp:1803 (discriminator 1)
    #4 /js/xpconnect/src/XPCWrappedNativeJSOps.cpp:983
    #5 /js/src/jscntxtinlines.h:282 (discriminator 3)
    #6 /js/src/vm/Interpreter.cpp:499
    #7 /js/src/vm/Interpreter.cpp:394
    #8 /js/src/vm/Interpreter.cpp:466 (discriminator 1)
    #9 /js/src/vm/Interpreter.cpp:512
    #10 /js/src/proxy/Wrapper.cpp:165 (discriminator 3)
    #11 /js/src/proxy/CrossCompartmentWrapper.cpp:353 (discriminator 1)
    #12 /js/src/proxy/Proxy.cpp:464 (discriminator 1)
    #13 /js/src/proxy/Proxy.cpp:716 (discriminator 1)
    #14 /js/src/jscntxtinlines.h:282 (discriminator 3)
    #15 /js/src/jit/BaselineIC.cpp:2347
    #16 0x3800de6f599f  (<unknown module>)
    #17 0x621000b8b6e7  (<unknown module>)
    #18 0x3800de957e30  (<unknown module>)
    #19 0x621000b884a7  (<unknown module>)
    #20 0x3800de6ea8a5  (<unknown module>)
    #21 /js/src/jit/BaselineJIT.cpp:160 (discriminator 2)
    #22 /js/src/jit/BaselineJIT.cpp:200 (discriminator 1)
    #23 /js/src/vm/Interpreter.cpp:384 (discriminator 1)
    #24 /js/src/vm/Interpreter.cpp:466 (discriminator 1)
    #25 /js/src/vm/Interpreter.cpp:512
    #26 /js/src/jsfun.cpp:1203 (discriminator 3)
    #27 /js/src/jscntxtinlines.h:282 (discriminator 3)
    #28 /js/src/vm/Interpreter.cpp:512
    #29 /js/src/proxy/Wrapper.cpp:165 (discriminator 3)

previously allocated by thread T0 here:
    #0 /llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:52
    #1 /xpcom/string/nsSubstring.cpp:217
    #2 /xpcom/string/nsTSubstring.cpp:710
    #3 /xpcom/string/nsTSubstring.cpp:748
    #4 /xpcom/string/nsReadableUtils.cpp:333
    #5 /dom/base/nsDocument.cpp:7313
    #6 /dom/base/nsDocument.cpp:7339
    #7 /objdir-ff-asan/dom/bindings/DocumentBinding.cpp:564 (discriminator 3)
    #8 /dom/bindings/BindingUtils.cpp:2843 (discriminator 3)
    #9 /js/src/jscntxtinlines.h:282 (discriminator 3)
    #10 /js/src/vm/Interpreter.cpp:512
    #11 /js/src/jsapi.cpp:2887 (discriminator 3)
    #12 /objdir-ff-asan/dist/include/jsapi.h:3421 (discriminator 3)
    #13 /js/xpconnect/wrappers/AddonWrapper.cpp:199 (discriminator 4)
    #14 /js/src/proxy/Proxy.cpp:325 (discriminator 4)
    #15 /js/src/vm/NativeObject.h:1441 (discriminator 4)
    #16 /js/src/vm/NativeObject.h:1441 (discriminator 4)
    #17 /js/src/vm/NativeObject.h:1441 (discriminator 4)
    #18 /js/src/vm/Interpreter.cpp:192 (discriminator 3)
    #19 /js/src/vm/Interpreter.cpp:394
    #20 /js/src/vm/Interpreter.cpp:466 (discriminator 1)
    #21 /js/src/vm/Interpreter.cpp:512
    #22 /js/src/proxy/Wrapper.cpp:165 (discriminator 3)
    #23 /js/src/proxy/CrossCompartmentWrapper.cpp:353 (discriminator 1)
    #24 /js/src/proxy/Proxy.cpp:464 (discriminator 1)
    #25 /js/src/proxy/Proxy.cpp:716 (discriminator 1)
    #26 /js/src/jscntxtinlines.h:282 (discriminator 3)
    #27 /js/src/vm/Interpreter.cpp:499
    #28 /js/src/vm/Interpreter.cpp:394
    #29 /js/src/vm/Interpreter.cpp:466 (discriminator 1)

Reporter

Comment 7

•

7 years ago

on m-c 35398cae65c1

Reporter

Comment 8

•

7 years ago

so, it's freed here:

https://dxr.mozilla.org/mozilla-central/rev/35398cae65c1/js/src/vm/String-inl.h#414
> inline void
> JSExternalString::finalize(js::FreeOp* fop)
> {
> ...
>     const JSStringFinalizer* fin = externalFinalizer();
>     fin->finalize(zone(), fin, const_cast<char16_t*>(rawTwoByteChars()));
> }

Reporter

Comment 9

•

7 years ago

now I'm re-building with --disable-optimize
(forgot to change that when getting trace in comment #6 :P

Reporter

Comment 10

•

7 years ago

trace with non-opt build

==23794==ERROR: AddressSanitizer: heap-use-after-free on address 0x61100066f5d8 at pc 0x0000004b16c5 bp 0x7ffd0b9092b0 sp 0x7ffd0b908a60
READ of size 2 at 0x61100066f5d8 thread T0
    #0 llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:411
    #1 objdir-ff-asan-debug/dist/include/mozilla/PodOperations.h:87
    #2 js/src/vm/String.cpp:537 (discriminator 1)
    #3 js/src/vm/String.cpp:576
    #4 js/src/vm/String.cpp:588
    #5 js/src/vm/String.h:1359 (discriminator 1)
    #6 objdir-ff-asan-debug/dist/include/jsfriendapi.h:871
    #7 dom/base/nsJSUtils.h:155 (discriminator 1)
    #8 objdir-ff-asan-debug/dist/include/mozilla/dom/BindingUtils.h:1898
    #9 objdir-ff-asan-debug/dom/bindings/ElementBinding.cpp:722 (discriminator 2)
    #10 dom/bindings/BindingUtils.cpp:2953 (discriminator 2)
    #11 js/src/jscntxtinlines.h:282 (discriminator 2)
    #12 js/src/vm/Interpreter.cpp:493
    #13 js/src/vm/Interpreter.cpp:499
    #14 js/src/vm/Interpreter.cpp:2954
    #15 js/src/vm/Interpreter.cpp:394
    #16 js/src/vm/Interpreter.cpp:466
    #17 js/src/vm/Interpreter.cpp:493
    #18 js/src/vm/Interpreter.cpp:512
    #19 js/src/jsfun.cpp:1269 (discriminator 4)
    #20 js/src/jscntxtinlines.h:282 (discriminator 2)
    #21 js/src/vm/Interpreter.cpp:493
    #22 js/src/vm/Interpreter.cpp:512
    #23 js/src/proxy/Wrapper.cpp:165 (discriminator 3)
    #24 js/src/proxy/CrossCompartmentWrapper.cpp:353 (discriminator 1)
    #25 js/src/proxy/Proxy.cpp:464 (discriminator 1)
    #26 js/src/proxy/Proxy.cpp:716 (discriminator 1)
    #27 js/src/jscntxtinlines.h:282 (discriminator 2)
    #28 js/src/vm/Interpreter.cpp:493
    #29 js/src/vm/Interpreter.cpp:499
    #30 js/src/vm/Interpreter.cpp:2954
    #31 js/src/vm/Interpreter.cpp:394
    #32 js/src/vm/Interpreter.cpp:466
    #33 js/src/vm/Interpreter.cpp:493
    #34 js/src/vm/Interpreter.cpp:512
    #35 js/src/jsapi.cpp:2828 (discriminator 3)
    #36 js/xpconnect/src/XPCWrappedJSClass.cpp:1214 (discriminator 4)
    #37 js/xpconnect/src/XPCWrappedJS.cpp:613 (discriminator 1)
    #38 xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp:120
    #39 xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp:?

0x61100066f5d8 is located 24 bytes inside of 256-byte region [0x61100066f5c0,0x61100066f6c0)
freed by thread T0 here:
    #0 llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:38
    #1 xpcom/string/nsSubstring.cpp:201
    #2 js/xpconnect/src/XPCString.cpp:80
    #3 js/src/vm/String-inl.h:414 (discriminator 2)
    #4 js/src/vm/String.cpp:1085 (discriminator 2)
    #5 js/src/vm/String.cpp:1064 (discriminator 1)
    #6 js/src/jsapi.cpp:5360
    #7 js/xpconnect/src/XPCConvert.cpp:634 (discriminator 1)
    #8 js/xpconnect/src/XPCWrappedNative.cpp:1803 (discriminator 1)
    #9 js/xpconnect/src/XPCWrappedNativeJSOps.cpp:983
    #10 js/src/jscntxtinlines.h:282 (discriminator 2)
    #11 js/src/vm/Interpreter.cpp:493
    #12 js/src/vm/Interpreter.cpp:499
    #13 js/src/vm/Interpreter.cpp:2954
    #14 js/src/vm/Interpreter.cpp:394
    #15 js/src/vm/Interpreter.cpp:466
    #16 js/src/vm/Interpreter.cpp:493
    #17 js/src/vm/Interpreter.cpp:512
    #18 js/src/proxy/Wrapper.cpp:165 (discriminator 3)
    #19 js/src/proxy/CrossCompartmentWrapper.cpp:353 (discriminator 1)
    #20 js/src/proxy/Proxy.cpp:464 (discriminator 1)
    #21 js/src/proxy/Proxy.cpp:716 (discriminator 1)
    #22 js/src/jscntxtinlines.h:282 (discriminator 2)
    #23 js/src/vm/Interpreter.cpp:493
    #24 js/src/vm/Interpreter.cpp:499
    #25 js/src/vm/Interpreter.cpp:2954
    #26 js/src/vm/Interpreter.cpp:394
    #27 js/src/vm/Interpreter.cpp:466
    #28 js/src/vm/Interpreter.cpp:493
    #29 js/src/vm/Interpreter.cpp:512

previously allocated by thread T0 here:
    #0 llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:52
    #1 xpcom/string/nsSubstring.cpp:217
    #2 xpcom/string/nsTSubstring.cpp:172
    #3 xpcom/string/nsTSubstring.cpp:710
    #4 xpcom/string/nsTSubstring.cpp:748
    #5 xpcom/string/nsReadableUtils.cpp:354
    #6 xpcom/string/nsReadableUtils.cpp:333
    #7 xpcom/string/nsReadableUtils.cpp:138
    #8 dom/base/nsDocument.cpp:7313
    #9 dom/base/nsDocument.cpp:7339
    #10 objdir-ff-asan-debug/dom/bindings/DocumentBinding.cpp:564 (discriminator 3)
    #11 dom/bindings/BindingUtils.cpp:2843 (discriminator 2)
    #12 js/src/jscntxtinlines.h:282 (discriminator 2)
    #13 js/src/vm/Interpreter.cpp:493
    #14 js/src/vm/Interpreter.cpp:512
    #15 js/src/jsapi.cpp:2887 (discriminator 3)
    #16 objdir-ff-asan-debug/dist/include/jsapi.h:3421 (discriminator 3)
    #17 js/xpconnect/wrappers/XrayWrapper.cpp:2298 (discriminator 4)
    #18 js/xpconnect/wrappers/AddonWrapper.cpp:199 (discriminator 4)
    #19 js/src/proxy/Proxy.cpp:325 (discriminator 4)
    #20 js/src/vm/NativeObject.h:1441 (discriminator 4)
    #21 js/src/proxy/Proxy.cpp:321 (discriminator 4)
    #22 js/src/vm/NativeObject.h:1441 (discriminator 4)
    #23 js/src/proxy/Proxy.cpp:321 (discriminator 4)
    #24 js/src/vm/NativeObject.h:1441 (discriminator 4)
    #25 js/src/jsobj.h:845 (discriminator 4)
    #26 js/src/vm/Interpreter.cpp:4307 (discriminator 4)
    #27 js/src/vm/Interpreter.cpp:192 (discriminator 3)
    #28 js/src/vm/Interpreter.cpp:2671 (discriminator 6)
    #29 js/src/vm/Interpreter.cpp:394

SUMMARY: AddressSanitizer: heap-use-after-free (llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:411)
Shadow bytes around the buggy address:
  0x0c22800c5e60: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c22800c5e70: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c22800c5e80: fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa fa
  0x0c22800c5e90: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c22800c5ea0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0c22800c5eb0: fa fa fa fa fa fa fa fa fd fd fd[fd]fd fd fd fd
  0x0c22800c5ec0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c22800c5ed0: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x0c22800c5ee0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c22800c5ef0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa
  0x0c22800c5f00: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb

Updated

•

7 years ago

Group: core-security → javascript-core-security

Reporter

Comment 11

•

7 years ago

the read happens when I try to access a rope, while calling setAttribute with it.

and the memory block is freed while calling a functio with an external string.

the string is created by documentURI getter.


==23794==ERROR: AddressSanitizer: heap-use-after-free on address 0x61100066f5d8 at pc 0x0000004b16c5 bp 0x7ffd0b9092b0 sp 0x7ffd0b908a60
READ of size 2 at 0x61100066f5d8 thread T0
    #0 llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:411
    #1 objdir-ff-asan-debug/dist/include/mozilla/PodOperations.h:87
> template<typename T>
> static MOZ_ALWAYS_INLINE void
> PodAssign(T* aDst, const T* aSrc)
> {
> ...
>   memcpy(reinterpret_cast<char*>(aDst), reinterpret_cast<const char*>(aSrc),
>          sizeof(T));

    #2 js/src/vm/String.cpp:537 (discriminator 1)
> template<JSRope::UsingBarrier b, typename CharT>
> JSFlatString*
> JSRope::flattenInternal(JSContext* maybecx)
> {
> ...
>     visit_right_child: {
>         JSString& right = *str->d.s.u3.right;
> ...
>         CopyChars(pos, right.asLinear());

    #3 js/src/vm/String.cpp:576
> template<JSRope::UsingBarrier b>
> JSFlatString*
> JSRope::flattenInternal(JSContext* maybecx)
> {
>     if (hasTwoByteChars())
>         return flattenInternal<b, char16_t>(maybecx);

    #4 js/src/vm/String.cpp:588
> JSFlatString*
> JSRope::flatten(JSContext* maybecx)
> {
> ...
>     if (zone()->needsIncrementalBarrier())
>         return flattenInternal<WithIncrementalBarrier>(maybecx);

    #5 js/src/vm/String.h:1359 (discriminator 1)
> MOZ_ALWAYS_INLINE JSLinearString*
> JSString::ensureLinear(JSContext* cx)
> {
>     return isLinear()
>            ? &asLinear()
>            : asRope().flatten(cx);

    #6 objdir-ff-asan-debug/dist/include/jsfriendapi.h:871
> MOZ_ALWAYS_INLINE JSLinearString*
> StringToLinearString(JSContext* cx, JSString* str)
> {
> ...
>     if (MOZ_UNLIKELY((s->flags & String::TYPE_FLAGS_MASK) == String::ROPE_FLAGS))
>         return StringToLinearStringSlow(cx, str);

    #7 dom/base/nsJSUtils.h:155 (discriminator 1)
> template<typename T>
> inline bool
> AssignJSString(JSContext *cx, T &dest, JSString *s)
> {
> ...
>   return js::CopyStringChars(cx, dest.BeginWriting(), s, len);

    #8 objdir-ff-asan-debug/dist/include/mozilla/dom/BindingUtils.h:1898
> template<typename T>
> static inline bool
> ConvertJSValueToString(JSContext* cx, JS::Handle<JS::Value> v,
>                        StringificationBehavior nullBehavior,
>                        StringificationBehavior undefinedBehavior,
>                        T& result)
> {
> ...
>   return AssignJSString(cx, result, s);

    #9 objdir-ff-asan-debug/dom/bindings/ElementBinding.cpp:722 (discriminator 2)
> static bool
> setAttribute(JSContext* cx, JS::Handle<JSObject*> obj, mozilla::dom::Element* self, const JSJitMethodCallArgs& args)
> {
> ...
>   if (!ConvertJSValueToString(cx, args[1], eStringify, eStringify, arg1)) {
>     return false;
>   }

    #10 dom/bindings/BindingUtils.cpp:2953 (discriminator 2)
> bool
> GenericBindingMethod(JSContext* cx, unsigned argc, JS::Value* vp)
> {
> ...
>   bool ok = method(cx, obj, self, JSJitMethodCallArgs(args));

    #11 js/src/jscntxtinlines.h:282 (discriminator 2)
    #12 js/src/vm/Interpreter.cpp:493
    #13 js/src/vm/Interpreter.cpp:499
    #14 js/src/vm/Interpreter.cpp:2954
    #15 js/src/vm/Interpreter.cpp:394
    #16 js/src/vm/Interpreter.cpp:466
    #17 js/src/vm/Interpreter.cpp:493
    #18 js/src/vm/Interpreter.cpp:512
    #19 js/src/jsfun.cpp:1269 (discriminator 4)
    #20 js/src/jscntxtinlines.h:282 (discriminator 2)
    #21 js/src/vm/Interpreter.cpp:493
    #22 js/src/vm/Interpreter.cpp:512
    #23 js/src/proxy/Wrapper.cpp:165 (discriminator 3)
    #24 js/src/proxy/CrossCompartmentWrapper.cpp:353 (discriminator 1)
    #25 js/src/proxy/Proxy.cpp:464 (discriminator 1)
    #26 js/src/proxy/Proxy.cpp:716 (discriminator 1)
    #27 js/src/jscntxtinlines.h:282 (discriminator 2)
    #28 js/src/vm/Interpreter.cpp:493
    #29 js/src/vm/Interpreter.cpp:499
    #30 js/src/vm/Interpreter.cpp:2954
    #31 js/src/vm/Interpreter.cpp:394
    #32 js/src/vm/Interpreter.cpp:466
    #33 js/src/vm/Interpreter.cpp:493
    #34 js/src/vm/Interpreter.cpp:512
    #35 js/src/jsapi.cpp:2828 (discriminator 3)
    #36 js/xpconnect/src/XPCWrappedJSClass.cpp:1214 (discriminator 4)
    #37 js/xpconnect/src/XPCWrappedJS.cpp:613 (discriminator 1)
    #38 xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp:120
    #39 xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp:?

0x61100066f5d8 is located 24 bytes inside of 256-byte region [0x61100066f5c0,0x61100066f6c0)
freed by thread T0 here:
    #0 llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:38
    #1 xpcom/string/nsSubstring.cpp:201
> void
> nsStringBuffer::Release()
> {
>   int32_t count = --mRefCount;
>   NS_LOG_RELEASE(this, count, "nsStringBuffer");
>   if (count == 0) {
>     STRING_STAT_INCREMENT(Free);
>     free(this); // we were allocated with |malloc|

    #2 js/xpconnect/src/XPCString.cpp:80
> // static
> void
> XPCStringConvert::FinalizeDOMString(JS::Zone* zone, const JSStringFinalizer* fin, char16_t* chars)
> {
>     nsStringBuffer* buf = nsStringBuffer::FromData(chars);
> ...
>     buf->Release();

    #3 js/src/vm/String-inl.h:414 (discriminator 2)
> inline void
> JSExternalString::finalize(js::FreeOp* fop)
> {
> ...
>     fin->finalize(zone(), fin, const_cast<char16_t*>(rawTwoByteChars()));

    #4 js/src/vm/String.cpp:1085 (discriminator 2)
> JSFlatString*
> JSExternalString::ensureFlat(JSContext* cx)
> {
> ...
>     // Release the external chars.
>     finalize(cx->runtime()->defaultFreeOp());

    #5 js/src/vm/String.cpp:1064 (discriminator 1)
> JSFlatString*
> JSString::ensureFlat(JSContext* cx)
> {
> ...
>     return asExternal().ensureFlat(cx);

    #6 js/src/jsapi.cpp:5360
> extern JS_PUBLIC_API(JSFlatString*)
> JS_FlattenString(JSContext* cx, JSString* str)
> {
> ...
>     JSFlatString* flat = str->ensureFlat(cx);

    #7 js/xpconnect/src/XPCConvert.cpp:634 (discriminator 1)
> // static
> bool
> XPCConvert::JSData2Native(void* d, HandleValue s,
>                           const nsXPTType& type,
>                           const nsID* iid,
>                           nsresult* pErr)
> {
> ...
>     switch (type.TagPart()) {
> ...
>     case nsXPTType::T_UTF8STRING:
>     {
> ...
>         JSFlatString* flat = JS_FlattenString(cx, str);

    #8 js/xpconnect/src/XPCWrappedNative.cpp:1803 (discriminator 1)
> bool
> CallMethodHelper::ConvertIndependentParam(uint8_t i)
> {
> ...
>     if (!XPCConvert::JSData2Native(&dp->val, src, type, &param_iid, &err)) {

    #9 js/xpconnect/src/XPCWrappedNativeJSOps.cpp:983
> bool
> XPC_WN_CallMethod(JSContext* cx, unsigned argc, Value* vp)
> {
> ...
>     return XPCWrappedNative::CallMethod(ccx);

    #10 js/src/jscntxtinlines.h:282 (discriminator 2)
> MOZ_ALWAYS_INLINE bool
> CallJSNative(JSContext* cx, Native native, const CallArgs& args)
> {
> ...
>     bool ok = native(cx, args.length(), args.base());

    #11 js/src/vm/Interpreter.cpp:493
> static bool
> InternalCall(JSContext* cx, const AnyInvokeArgs& args)
> {
> ...
>     return InternalCallOrConstruct(cx, args, NO_CONSTRUCT);

    #12 js/src/vm/Interpreter.cpp:499
> bool
> js::CallFromStack(JSContext* cx, const CallArgs& args)
> {
>     return InternalCall(cx, static_cast<const AnyInvokeArgs&>(args));

    #13 js/src/vm/Interpreter.cpp:2954
> CASE(JSOP_NEW)
> CASE(JSOP_CALL)
> CASE(JSOP_CALLITER)
> CASE(JSOP_SUPERCALL)
> CASE(JSOP_FUNCALL)
> {
> ...
>             if (!CallFromStack(cx, args))

    #14 js/src/vm/Interpreter.cpp:394
    #15 js/src/vm/Interpreter.cpp:466
    #16 js/src/vm/Interpreter.cpp:493
    #17 js/src/vm/Interpreter.cpp:512
    #18 js/src/proxy/Wrapper.cpp:165 (discriminator 3)
    #19 js/src/proxy/CrossCompartmentWrapper.cpp:353 (discriminator 1)
    #20 js/src/proxy/Proxy.cpp:464 (discriminator 1)
    #21 js/src/proxy/Proxy.cpp:716 (discriminator 1)
    #22 js/src/jscntxtinlines.h:282 (discriminator 2)
    #23 js/src/vm/Interpreter.cpp:493
    #24 js/src/vm/Interpreter.cpp:499
    #25 js/src/vm/Interpreter.cpp:2954
    #26 js/src/vm/Interpreter.cpp:394
    #27 js/src/vm/Interpreter.cpp:466
    #28 js/src/vm/Interpreter.cpp:493
    #29 js/src/vm/Interpreter.cpp:512

previously allocated by thread T0 here:
    #0 llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:52
    #1 xpcom/string/nsSubstring.cpp:217
> already_AddRefed<nsStringBuffer>
> nsStringBuffer::Alloc(size_t aSize)
> {
> ...
>     (nsStringBuffer*)malloc(sizeof(nsStringBuffer) + aSize);

    #2 xpcom/string/nsTSubstring.cpp:172
> bool
> nsTSubstring_CharT::MutatePrep(size_type aCapacity, char_type** aOldData,
>                                uint32_t* aOldFlags)
> {
> ...
>     nsStringBuffer* newHdr =
>       nsStringBuffer::Alloc(storageSize).take();

    #3 xpcom/string/nsTSubstring.cpp:710
> bool
> nsTSubstring_CharT::SetCapacity(size_type aCapacity, const fallible_t&)
> {
> ...
>   if (!MutatePrep(aCapacity, &oldData, &oldFlags)) {

    #4 xpcom/string/nsTSubstring.cpp:748
> bool
> nsTSubstring_CharT::SetLength(size_type aLength, const fallible_t& aFallible)
> {
>   if (!SetCapacity(aLength, aFallible)) {

    #5 xpcom/string/nsReadableUtils.cpp:354
> bool
> AppendUTF8toUTF16(const nsACString& aSource, nsAString& aDest,
>                   const mozilla::fallible_t& aFallible)
> {
> ...
>     if (!aDest.SetLength(old_dest_length + count, aFallible)) {

    #6 xpcom/string/nsReadableUtils.cpp:333
> void
> AppendUTF8toUTF16(const nsACString& aSource, nsAString& aDest)
> {
>   if (!AppendUTF8toUTF16(aSource, aDest, mozilla::fallible)) {

    #7 xpcom/string/nsReadableUtils.cpp:138
> void
> CopyUTF8toUTF16(const nsACString& aSource, nsAString& aDest)
> {
>   aDest.Truncate();
>   AppendUTF8toUTF16(aSource, aDest);

    #8 dom/base/nsDocument.cpp:7313
> nsresult
> nsIDocument::GetDocumentURI(nsString& aDocumentURI) const
> {
>   if (mDocumentURI) {
>     nsAutoCString uri;
>     nsresult rv = mDocumentURI->GetSpec(uri);
>     NS_ENSURE_SUCCESS(rv, rv);
> 
>     CopyUTF8toUTF16(uri, aDocumentURI);

    #9 dom/base/nsDocument.cpp:7339
> void
> nsIDocument::GetDocumentURIFromJS(nsString& aDocumentURI, CallerType aCallerType,
>                                   ErrorResult& aRv) const
> {
>   if (!mChromeXHRDocURI || aCallerType != CallerType::System) {
>     aRv = GetDocumentURI(aDocumentURI);

    #10 objdir-ff-asan-debug/dom/bindings/DocumentBinding.cpp:564 (discriminator 3)
> static bool
> get_documentURI(JSContext* cx, JS::Handle<JSObject*> obj, nsIDocument* self, JSJitGetterCallArgs args)
> {
> ...
>   self->GetDocumentURIFromJS(result, nsContentUtils::IsSystemCaller(cx) ? CallerType::System : CallerType::NonSystem, rv);

    #11 dom/bindings/BindingUtils.cpp:2843 (discriminator 2)
    #12 js/src/jscntxtinlines.h:282 (discriminator 2)
    #13 js/src/vm/Interpreter.cpp:493
    #14 js/src/vm/Interpreter.cpp:512
    #15 js/src/jsapi.cpp:2887 (discriminator 3)
    #16 objdir-ff-asan-debug/dist/include/jsapi.h:3421 (discriminator 3)
    #17 js/xpconnect/wrappers/XrayWrapper.cpp:2298 (discriminator 4)
    #18 js/xpconnect/wrappers/AddonWrapper.cpp:199 (discriminator 4)
    #19 js/src/proxy/Proxy.cpp:325 (discriminator 4)
    #20 js/src/vm/NativeObject.h:1441 (discriminator 4)
    #21 js/src/proxy/Proxy.cpp:321 (discriminator 4)
    #22 js/src/vm/NativeObject.h:1441 (discriminator 4)
    #23 js/src/proxy/Proxy.cpp:321 (discriminator 4)
    #24 js/src/vm/NativeObject.h:1441 (discriminator 4)
    #25 js/src/jsobj.h:845 (discriminator 4)
    #26 js/src/vm/Interpreter.cpp:4307 (discriminator 4)
    #27 js/src/vm/Interpreter.cpp:192 (discriminator 3)
    #28 js/src/vm/Interpreter.cpp:2671 (discriminator 6)
    #29 js/src/vm/Interpreter.cpp:394

====


what I did on the documentURI is, patterm match on the string with RegExp and get each match result (that is dependent string):

>     m = path.match(/^([A-Za-z0-9\-]+):\/\/([^\/]*)\/([^\?]*)(\?(((?:.|\r|\n)*)*))?$/);
>     if (m) {
>       /*
>        * http://host1/hoge/fuga.html
>        * http://host1/hoge/fuga.cgi?name=value
>        */
>       info.type = arPathType.URI;
>       info.sep = "/";
>       info.scheme = m[1];
>       info.host = m[2];
>       let m2 = info.host.match(/^(.+):([0-9]+)$/);
>       if (m2) {
>         info.host = m2[1];
>         info.port = m2[2];
>       }
>       info.pathComponents = m[3].split(info.sep);
>       if (m[4]) {
>         info.query = m[5];
>       }
>       return info;
>     }

and concatenate it again (so it should be rope):

>       case arPathType.URI: {
>         result = (info.scheme + "://" + info.host
>                   + ((info.port !== undefined) ? (":" + info.port) : "")
>                   + info.sep
>                   + info.pathComponents.join(info.sep)
>                   + ((info.query !== undefined) ? ("?" + info.query) : ""));
>         break;
>       }

and pass it to setAttribute of base element:

>     let node = targetDocument.createElement("base");
>     node.setAttribute("href", baseDir);
>     return node;

I haven't yet spotted where the documentURI is passed to some method that uses XPC_WN_CallMethod tho, the documentURI string should be passed to many functions, and in that case this matches to the trace.

So, this should be an issue around dependent string and external string.

Reporter

Comment 12

•

7 years ago

I won't be able to work on this shortly.
jandem, can you take a look?

Flags: needinfo?(jdemooij)

Assignee

Updated

•

7 years ago

Flags: needinfo?(jdemooij)

Keywords: sec-critical

Assignee

Updated

•

7 years ago

Flags: needinfo?(jdemooij)

Assignee

Comment 13

•

7 years ago

Attached patch Part 1 - Fix — Details — Splinter Review

This is bad. It's possible to create a dependent string with an external string as base string. Then when we flatten the external string, the dependent string still points to the potentially-freed external chars => UAF.

The simplest and safest fix is to call ensureFlat when we create a dependent string and the base string is external. JIT code can also create dependent strings so I had to fix these places too to take the slow path.

I also changed AutoStableStringChars to call ensureFlat if the string is external.

The right long-term fix is to remove flat strings (bug 1330776) so this whole problem goes away.

Assignee: nobody → jdemooij

Status: NEW → ASSIGNED

Flags: needinfo?(jdemooij)

Attachment #8846672 - Flags: review?(jwalden+bmo)

Attachment #8846672 - Flags: review?(hv1989)

Assignee

Comment 14

•

7 years ago

Attached patch Part 2 - Test, comments — Details — Splinter Review

Attachment #8846675 - Flags: review?(jwalden+bmo)

Liz Henry (:lizzard) (relman/hg->git project)

Assignee

Comment 15

•

7 years ago

Given pwn2own in a few days, I'd appreciate quick reviews. Maybe we can take a fix for this in a dot release.

Summary: Use-after-free around hostname in URL when saving a document with UnMHT extension → Use-after-free when creating dependent strings with an external base string

Comment 16

•

7 years ago

Tracking since this is rated sec-critical.

tracking-firefox52: --- → ?

tracking-firefox53: --- → +

tracking-firefox54: --- → +

tracking-firefox55: --- → +

tracking-firefox-esr52: --- → ?

Hannes Verschore [:h4writer]

Comment 17

•

7 years ago

Comment on attachment 8846672 [details] [diff] [review]
Part 1 - Fix

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

I see CreateDependentString::generate in nightly, which is only used by PrepareAndExecuteRegExp. So it might be okay. But can we put some debug assertions in there that just to be sure. That people don't start reusing that code?
Thanks for the quick action and I also hope we can spin a new FF52 release? Pwn2own is starting 15th of March!

Attachment #8846672 - Flags: review?(hv1989) → review+

Liz Henry (:lizzard) (relman/hg->git project)

Comment 18

•

7 years ago

From what I hear pwn2own uses a VM that they prepare beforehand. It's not impossible we could get a dot release out the door before the 15th, but they have likely already made and distributed their VM with 52.0 on it.

Jeff Walden [:Waldo]

Comment 19

•

7 years ago

Comment on attachment 8846672 [details] [diff] [review]
Part 1 - Fix

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

Looks good.  I'm not 100% confident that, in auditing the existing code *not* changed by this patch, everything is copacetic.  But I guess that's at least part of why multiple people are looking.

Attachment #8846672 - Flags: review?(jwalden+bmo) → review+

Jeff Walden [:Waldo]

Updated

•

7 years ago

Attachment #8846675 - Flags: review?(jwalden+bmo) → review+

Comment 20

•

7 years ago

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #18)
> From what I hear pwn2own uses a VM that they prepare beforehand. It's not
> impossible we could get a dot release out the door before the 15th, but they
> have likely already made and distributed their VM with 52.0 on it.

We don't know when they do that. Could be the morning of the conference; more likely it's earlier but I don't know how much earlier. But there's also a check after the contestant "wins" where the dev team could show the bug is already fixed which can affect the amount won by the contestant, though not the headlines that Firefox got pwned.

status-firefox-esr45: --- → unaffected

Comment 21

•

7 years ago

I don't think we should try to rush a point release for this when we know we'll almost certainly have to do one at the end of the week. Our own fuzzers haven't found this regression in the past month so we can hope the contest participants already had their exploit in the bag and didn't find it either. If that turns out to be a bad guess we can be sad, but at least it will make shipping a fix that much faster. Either way we should include this fix in that release because I'd hate to land the fix and have it sit in the tree for weeks.

tracking-firefox52: ? → +

tracking-firefox-esr52: ? → 52+

Hannes Verschore [:h4writer]

Assignee

Comment 22

•

7 years ago

Sure, I didn't mean we should rush a point release for this before pwn2own, but since there will almost certainly be one later this week it's good to have a patch ready.

Updated

•

7 years ago

Priority: -- → P1

Reporter

Comment 23

•

7 years ago

maybe I could ask bug bounty for this?

Assignee

Updated

•

7 years ago

Flags: sec-bounty?

Assignee

Comment 24

•

7 years ago

Attached patch Part 3 - Add debug check to CreateDependentString::generate — Details — Splinter Review

This adds some debug-only code to CreateDependentString::generate to check the string is not a rope or an external string.

Attachment #8847101 - Flags: review?(hv1989)

Al Billings [:abillings - ex-MoCo]

Assignee

Comment 25

•

7 years ago

Comment on attachment 8846672 [details] [diff] [review]
Part 1 - Fix

[Security approval request comment]
> How easily could an exploit be constructed based on the patch?
Not too difficult I'm afraid. What helps a little is that JS strings are immutable and I *think* an attacker could use this UAF to read memory but not to write it (but it's bad anyway of course).

> 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?
52+.

> If not all supported branches, which bug introduced the flaw?
Bug 1330593.

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

> How likely is this patch to cause regressions; how much testing does it need?
Unlikely. It could regress memory usage or performance a bit in some cases but I don't expect it to be noticeable.

Attachment #8846672 - Flags: sec-approval?

Comment 26

•

7 years ago

sec-approval+ for checkin to trunk on March 21, two weeks into the current dev cycle. 
Please nominate patches for affected branches as well, including the ESR ones.

status-firefox52: affected → wontfix

tracking-firefox52: + → ---

tracking-firefox-esr52: 52+ → 53+

Whiteboard: [checkin on 3/21]

Al Billings [:abillings - ex-MoCo]

Updated

•

7 years ago

Attachment #8846672 - Flags: sec-approval? → sec-approval+

Hannes Verschore [:h4writer]

Comment 27

•

7 years ago

Comment on attachment 8847101 [details] [diff] [review]
Part 3 - Add debug check to CreateDependentString::generate

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

Thanks!

Attachment #8847101 - Flags: review?(hv1989) → review+

Assignee

Comment 28

•

7 years ago

I'll land this one myself.

Flags: needinfo?(jdemooij)

Assignee

Updated

•

7 years ago

Blocks: 1348644

Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)

Assignee

Comment 29

•

7 years ago

Part 1:

https://hg.mozilla.org/integration/mozilla-inbound/rev/8c3bb39e71b1

Flags: needinfo?(jdemooij)

Whiteboard: [checkin on 3/21]

Comment 30

•

7 years ago

https://hg.mozilla.org/mozilla-central/rev/8c3bb39e71b1

Status: ASSIGNED → RESOLVED

Closed: 7 years ago

status-firefox55: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla55

Liz Henry (:lizzard) (relman/hg->git project)

Assignee

Comment 31

•

7 years ago

Comment on attachment 8846672 [details] [diff] [review]
Part 1 - Fix

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1330593.
[User impact if declined]: Security issues, crashes.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Not very risky.
[Why is the change risky/not risky?]: We will flatten/copy external strings in more cases so there's some memory usage and perf risk, but I don't expect it to be a problem.
[String changes made/needed]: None.

Attachment #8846672 - Flags: approval-mozilla-esr52?

Attachment #8846672 - Flags: approval-mozilla-beta?

Attachment #8846672 - Flags: approval-mozilla-aurora?

Comment 33

•

7 years ago

Comment on attachment 8846672 [details] [diff] [review]
Part 1 - Fix

Let's uplift now to aurora and beta (But hold off for ESR52 until next week)
This should make it into 53 beta 7.

Attachment #8846672 - Flags: approval-mozilla-beta?

Attachment #8846672 - Flags: approval-mozilla-beta+

Attachment #8846672 - Flags: approval-mozilla-aurora?

Attachment #8846672 - Flags: approval-mozilla-aurora+

Ryan VanderMeulen [:RyanVM]

Comment 34

•

7 years ago

uplift

https://hg.mozilla.org/releases/mozilla-aurora/rev/286da438411a
https://hg.mozilla.org/releases/mozilla-beta/rev/41a40a9d8768

status-firefox53: affected → fixed

status-firefox54: affected → fixed

Ryan VanderMeulen [:RyanVM]

Comment 35

•

7 years ago

Backed out from Beta for bustage.
https://hg.mozilla.org/releases/mozilla-beta/rev/f2b55ce343c6

https://treeherder.mozilla.org/logviewer.html#?job_id=86310432&repo=mozilla-beta

status-firefox53: fixed → affected

Flags: needinfo?(jdemooij)

Assignee

Comment 36

•

7 years ago

uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/df0808040376f8e12f1d110cf8855c7666476291

status-firefox53: affected → fixed

Julien Cristau [:jcristau]

Assignee

Updated

•

7 years ago

Flags: needinfo?(jdemooij)

Comment 37

•

7 years ago

Comment on attachment 8846672 [details] [diff] [review]
Part 1 - Fix

fix uaf regression in esr52

Attachment #8846672 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+

Ryan VanderMeulen [:RyanVM]

Comment 38

•

7 years ago

uplift

https://hg.mozilla.org/releases/mozilla-esr52/rev/ac0ca89b5a6b

status-firefox-esr52: affected → fixed

Updated

•

7 years ago

Group: javascript-core-security → core-security-release

Comment 39

•

7 years ago

(In reply to Tooru Fujisawa [:arai] from comment #23)
> maybe I could ask bug bounty for this?

I'm sorry, Tooru, but given the amount of your contribution to the JavaScript engine, including the specific files patched by this bug, we can't award you bounties in that area.

Flags: sec-bounty? → sec-bounty-

Al Billings [:abillings - ex-MoCo]

Comment 40

•

7 years ago

reference for comment 39: https://www.mozilla.org/en-US/security/client-bug-bounty/

Updated

•

7 years ago

Whiteboard: [adv-main53+][adv-esr52.1+]

Andrei Vaida [:avaida]

Comment 41

•

7 years ago

Setting qe-verify- based on Jan's assessment on manual testing needs and the fact that this fix has automated coverage (see Comment 31).

Flags: qe-verify-

Comment 42

•

7 years ago

jandem: web content can't directly create nsExternalString, can they? We do in chrome code for sure (see location bar related dupes of this bug) but if web content can't directly induce those chrome paths we should probably call this sec-high rather than sec-critical.

Flags: needinfo?(jdemooij)

Assignee

Comment 43

•

7 years ago

(In reply to Daniel Veditz [:dveditz] from comment #42)
> jandem: web content can't directly create nsExternalString, can they? We do
> in chrome code for sure (see location bar related dupes of this bug) but if
> web content can't directly induce those chrome paths we should probably call
> this sec-high rather than sec-critical.

Unfortunately web content *can* create external strings. When JS calls a DOM API that returns a string it's usually an external string..

Flags: needinfo?(jdemooij)