1192350 - null crash in XMLHttpRequest::Open()

Status: VERIFIED FIXED

(Core :: DOM: Workers, defect)

Description

[Kamil Jozwiak [:kjozwiak]]

+++ This bug was initially created as a clone of Bug #1185820 +++

Firefox version:42.0a1 (2015-07-16)
OS: Windows 7 64 bit

Steps to reproduce: 
1. Run server side script Uaf_XMLHttpRequest_Open.js in Node.js (node Uaf_XMLHttpRequest_Open.js).
2. Enter http://localhost:12345 in Firefox browser.
3. Firefox crashes in XMLHttpRequest::Open():

The original UAF was fixed but the test case still crashes with the following null crash:

Using the following asan m-c build:
- http://inbound-archive.pub.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1438856161/

=================================================================
==4819==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000054 (pc 0x7fcdab001ac5 sp 0x7fcd848d5bc0 bp 0x7fcd848d5c50 T32)
ASAN:SIGSEGV
==4819==AddressSanitizer: while reporting a bug found another one.Ignoring.
    #0 0x7fcdab001ac4 in Open XMLHttpRequest.cpp:1970
    #1 0x7fcda93bb518 in open XMLHttpRequestBinding.cpp:2200
    #2 0x7fcda9c23967 in GenericBindingMethod BindingUtils.cpp:2599
    #3 0x7fcdaea118f3 in CallJSNative jscntxtinlines.h:235
    #4 0x7fcdaea608d4 in Interpret Interpreter.cpp:3035
    #5 0x7fcdaea32d97 in RunScript Interpreter.cpp:714
    #6 0x7fcdaea120c8 in Invoke Interpreter.cpp:791
    #7 0x7fcdae9b1ca3 in Invoke Interpreter.cpp:828
    #8 0x7fcdaf56fd7f in Call jsapi.cpp:4628
    #9 0x7fcda9762c9f in Call EventHandlerBinding.cpp:259
    #10 0x7fcdaa009669 in Call<nsISupports *> EventHandlerBinding.h:351
    #11 0x7fcda9fc590c in HandleEventSubType EventListenerManager.cpp:998
    #12 0x7fcda9fc7160 in HandleEventInternal EventListenerManager.cpp:1147
    #13 0x7fcda9fb6b11 in HandleEventTargetChain EventDispatcher.cpp:299
    #14 0x7fcda9fbafca in Dispatch EventDispatcher.cpp:635
    #15 0x7fcda9f93e25 in DispatchDOMEvent EventDispatcher.cpp:699
    #16 0x7fcdab00e947 in WorkerRun XMLHttpRequest.cpp:1427
    #17 0x7fcdaafab7a4 in Run WorkerRunnable.cpp:359
    #18 0x7fcda5d8a0d7 in ProcessNextEvent nsThread.cpp:867
    #19 0x7fcda5df831a in NS_ProcessNextEvent nsThreadUtils.cpp:277
    #20 0x7fcdaaf93347 in RunCurrentSyncLoop WorkerPrivate.cpp:6253
    #21 0x7fcdaafff3e5 in Run WorkerPrivate.h:1568
    #22 0x7fcdab001a84 in Open XMLHttpRequest.cpp:1961
    #23 0x7fcda93bb518 in open XMLHttpRequestBinding.cpp:2200
    #24 0x7fcda9c23967 in GenericBindingMethod BindingUtils.cpp:2599
    #25 0x7fcdaea118f3 in CallJSNative jscntxtinlines.h:235
    #26 0x7fcdaea608d4 in Interpret Interpreter.cpp:3035
    #27 0x7fcdaea32d97 in RunScript Interpreter.cpp:714
    #28 0x7fcdaea120c8 in Invoke Interpreter.cpp:791
    #29 0x7fcdae9b1ca3 in Invoke Interpreter.cpp:828
    #30 0x7fcdaf56fd7f in Call jsapi.cpp:4628
    #31 0x7fcda9762c9f in Call EventHandlerBinding.cpp:259
    #32 0x7fcdaa009669 in Call<nsISupports *> EventHandlerBinding.h:351
    #33 0x7fcda9fc590c in HandleEventSubType EventListenerManager.cpp:998
    #34 0x7fcda9fc7160 in HandleEventInternal EventListenerManager.cpp:1147
    #35 0x7fcda9fb6b11 in HandleEventTargetChain EventDispatcher.cpp:299
    #36 0x7fcda9fbafca in Dispatch EventDispatcher.cpp:635
    #37 0x7fcda9f93e25 in DispatchDOMEvent EventDispatcher.cpp:699
    #38 0x7fcdab0001d6 in DispatchPrematureAbortEvent XMLHttpRequest.cpp:1837
    #39 0x7fcdaafff95f in MaybeDispatchPrematureAbortEvents XMLHttpRequest.cpp:1769
    #40 0x7fcdab0010fb in Open XMLHttpRequest.cpp:1944
    #41 0x7fcda93bb518 in open XMLHttpRequestBinding.cpp:2200
    #42 0x7fcda9c23967 in GenericBindingMethod BindingUtils.cpp:2599
    #43 0x7fcdaea118f3 in CallJSNative jscntxtinlines.h:235
    #44 0x7fcdaea608d4 in Interpret Interpreter.cpp:3035
    #45 0x7fcdaea32d97 in RunScript Interpreter.cpp:714
    #46 0x7fcdaea120c8 in Invoke Interpreter.cpp:791
    #47 0x7fcdae9b1ca3 in Invoke Interpreter.cpp:828
    #48 0x7fcdaf56fd7f in Call jsapi.cpp:4628
    #49 0x7fcda9762c9f in Call EventHandlerBinding.cpp:259
    #50 0x7fcdaa009669 in Call<nsISupports *> EventHandlerBinding.h:351
    #51 0x7fcda9fc590c in HandleEventSubType EventListenerManager.cpp:998
    #52 0x7fcda9fc7160 in HandleEventInternal EventListenerManager.cpp:1147
    #53 0x7fcda9fb6b11 in HandleEventTargetChain EventDispatcher.cpp:299
    #54 0x7fcda9fbafca in Dispatch EventDispatcher.cpp:635
    #55 0x7fcda9f93e25 in DispatchDOMEvent EventDispatcher.cpp:699
    #56 0x7fcdab00e947 in WorkerRun XMLHttpRequest.cpp:1427
    #57 0x7fcdaafab7a4 in Run WorkerRunnable.cpp:359
    #58 0x7fcda5d8a0d7 in ProcessNextEvent nsThread.cpp:867
    #59 0x7fcda5df831a in NS_ProcessNextEvent nsThreadUtils.cpp:277
    #60 0x7fcdaaf89dd3 in DoRunLoop WorkerPrivate.cpp:5416
    #61 0x7fcdaaf21147 in Run RuntimeService.cpp:2866
    #62 0x7fcda5d8a0d7 in ProcessNextEvent nsThread.cpp:867
    #63 0x7fcda5df831a in NS_ProcessNextEvent nsThreadUtils.cpp:277
    #64 0x7fcda6669f58 in Run MessagePump.cpp:355
    #65 0x7fcda65f5d8c in RunInternal message_loop.cc:234
    #66 0x7fcda5d864f5 in ThreadFunc nsThread.cpp:360
    #67 0x7fcdb2d804b5 in _pt_root ptthread.c:212
    #68 0x7fcdb33bf181 in start_thread pthread_create.c:312 (discriminator 2)
    #69 0x7fcda38bd47c in clone clone.S:111

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV ??:0 ??
Thread T32 (DOM Worker) created by T0 (Web Content) here:
    #0 0x461855 in __interceptor_pthread_create _asan_rtl_
    #1 0x7fcdb2d7ce3d in _PR_CreateThread ptthread.c:453
    #2 0x7fcdb2d7c9ba in PR_CreateThread ptthread.c:544
    #3 0x7fcda5d87aed in Init nsThread.cpp:470
    #4 0x7fcdaaff8c5a in Create WorkerThread.cpp:90
    #5 0x7fcdaaef4f40 in ScheduleWorker RuntimeService.cpp:1744
    #6 0x7fcdaaef2234 in RegisterWorker RuntimeService.cpp:1583
    #7 0x7fcdaaf88762 in Constructor WorkerPrivate.cpp:4977
    #8 0x7fcdaaefb3b3 in CreateSharedWorkerFromLoadInfo RuntimeService.cpp:2490
    #9 0x7fcdaaefac4f in CreateSharedWorkerInternal RuntimeService.cpp:2441
    #10 0x7fcdaaf6f65a in CreateSharedWorker RuntimeService.h:157
    #11 0x7fcda8edd873 in _constructor SharedWorkerBinding.cpp:240
    #12 0x7fcdaea81aee in CallJSNative jscntxtinlines.h:235
    #13 0x7fcdaea608c0 in Interpret Interpreter.cpp:3032
    #14 0x7fcdaea32d97 in RunScript Interpreter.cpp:714
    #15 0x7fcdaea832b8 in ExecuteKernel Interpreter.cpp:955
    #16 0x7fcdaea83918 in Execute Interpreter.cpp:988
    #17 0x7fcdaf56e253 in Evaluate jsapi.cpp:4464
    #18 0x7fcdaf56ea6b in Evaluate jsapi.cpp:4491
    #19 0x7fcda823b2f4 in EvaluateString nsJSUtils.cpp:224
    #20 0x7fcda823bf51 in EvaluateString nsJSUtils.cpp:286
    #21 0x7fcda82bf60f in EvaluateScript nsScriptLoader.cpp:1143
    #22 0x7fcda82bcd45 in ProcessRequest nsScriptLoader.cpp:970
    #23 0x7fcda82b6893 in ProcessScriptElement nsScriptLoader.cpp:764
    #24 0x7fcda82b1eee in MaybeProcessScript nsScriptElement.cpp:142
    #25 0x7fcda7675da4 in operator-> nsIScriptElement.h:221
    #26 0x7fcda7674291 in RunFlushLoop nsHtml5TreeOpExecutor.cpp:487
    #27 0x7fcda767a66b in Run nsHtml5StreamParser.cpp:127
    #28 0x7fcda5d8a0d7 in ProcessNextEvent nsThread.cpp:867
    #29 0x7fcda5df831a in NS_ProcessNextEvent nsThreadUtils.cpp:277
    #30 0x7fcda6668e49 in Run MessagePump.cpp:95
    #31 0x7fcda65f5d8c in RunInternal message_loop.cc:234
    #32 0x7fcdab451417 in Run nsBaseAppShell.cpp:165
    #33 0x7fcdad297f32 in XRE_RunAppShell nsEmbedFunctions.cpp:785
    #34 0x7fcda65f5d8c in RunInternal message_loop.cc:234
    #35 0x7fcdad297629 in XRE_InitChildProcess nsEmbedFunctions.cpp:621
    #36 0x48d670 in content_process_main plugin-container.cpp:237
    #37 0x7fcda37e4ec4 in __libc_start_main libc-start.c:287

==4819==ABORTING

Comment 1

[Andrea Marchesini [:baku]]

Attachment: crash.patch

I have to admit that I'm not so sure about this approach.

Comment 2

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9caee7468e39

Comment 3

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

https://hg.mozilla.org/mozilla-central/rev/9caee7468e39

Comment 4

[Kamil Jozwiak [:kjozwiak]]

Reproduced the original issue using the following asan build:
- http://inbound-archive.pub.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1439936618/

=================================================================
==5789==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000054 (pc 0x7f6ced638c25 sp 0x7f6cc60f4ae0 bp 0x7f6cc60f4b70 T27)
    #0 0x7f6ced638c24 in Open XMLHttpRequest.cpp:1961
    #1 0x7f6ceb955828 in open XMLHttpRequestBinding.cpp:2200
    #2 0x7f6cec238407 in GenericBindingMethod BindingUtils.cpp:2599
    #3 0x7f6cf108b9d3 in CallJSNative jscntxtinlines.h:235
    #4 0x7f6cf10daca4 in Interpret Interpreter.cpp:3035
    #5 0x7f6cf10ad167 in RunScript Interpreter.cpp:714
    #6 0x7f6cf108c1a8 in Invoke Interpreter.cpp:791
    #7 0x7f6cf102b1c3 in Invoke Interpreter.cpp:828
    #8 0x7f6cf1c1677f in Call jsapi.cpp:4606
    #9 0x7f6cebdcec0f in Call EventHandlerBinding.cpp:259

etc.....

Went through verification using the following asan build:
- http://inbound-archive.pub.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1440151324/

Test Cases:

While the browser is contacting the poc via localhost:12345, used the following cases:

- left it running for about 5 minutes several times
- closed the tab running localhost:12345 (this usually reproduced the issue quickly)
- shutdown firefox while running localhost:12345
- closed the tab running localhost:12345 while opening several other websites via new tabs
- opened 8 tabs and connected to localhost:12345, quickly closed them all

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Is this worth backporting anywhere?

Comment 6

[Andrea Marchesini [:baku]]

Comment on attachment 8645754 [details] [diff] [review]
crash.patch

This patch should land everywhere bug 1185820 is landed.

[Approval Request Comment]
User impact if declined: a crash can occur.
Fix Landed on Version: m-i ?
Risk to taking this patch (and alternatives if risky): none. 
String or UUID changes made by this patch: none

Approval Request Comment
[Feature/regressing bug #]: XHR in workers
[Describe test coverage new/current, TreeHerder]: it's racy, not easy to write a test.
[Risks and why]: none
[String/UUID change made/needed]: none

RyanVM, do you agree with the 3 approval requests?

Comment 7

[Ryan VanderMeulen [:RyanVM]]

sure

Comment 8

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8645754 [details] [diff] [review]
crash.patch

The fix seems simple, should be safe to uplift to Aurora42, Beta42 and ESR38.3.0.

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Mahe, n-i'ing you on the b2g uplift request. Not sure if you do it or somebody else.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e975c8dbdb18

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/79f48c46fe39

Comment 12

[Mahendra Potharaju [:mahe]]

Comment on attachment 8645754 [details] [diff] [review]
crash.patch

Ryan, Please uplift this to b2g-v2.2, b2g-v2.2r, b2g-37

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr38/rev/1ccf5993227b

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/8c482fd90141
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/8c482fd90141
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/b76c8843149c

Comment 15

[Kamil Jozwiak [:kjozwiak]]

Went through verification using the following builds:

- http://inbound-archive.pub.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-aurora-linux64-asan/1441039608/
- http://inbound-archive.pub.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-beta-linux64-asan/1441055449/
- http://inbound-archive.pub.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-esr38-linux64/1441052631/

Went through the test cases outlined in comment # 4 with the above builds and couldn't reproduce the null crash.