<a class="header-button" href="https://bugzilla.mozilla.org/home" title="Go to home page"> Bugzilla

Assignee

Comment 2

•

9 years ago

I take this bug.

Assignee: nobody → amarchesini

Flags: needinfo?(amarchesini)

Boris Zbarsky [:bzbarsky]

Assignee

Comment 3

•

9 years ago

Attached patch crash2.patch (obsolete) — Details — Splinter Review

I use mProxy after releasing it. This is why we crash. This code can easily land on m-i because the other bug has just landed last week.

Attachment #8613543 - Flags: review?(ehsan)

Comment 5

•

9 years ago

[Tracking Requested - why for this release]: Copying tracking flags from bug 1169979.

tracking-firefox39: --- → ?

tracking-firefox40: --- → ?

tracking-firefox41: --- → ?

tracking-firefox-esr31: --- → ?

tracking-firefox-esr38: --- → ?

(no longer active)

Updated

•

9 years ago

Attachment #8613543 - Flags: review?(ehsan) → review+

Assignee

Comment 6

•

9 years ago

Comment on attachment 8613543 [details] [diff] [review] crash2.patch [Security approval request comment] How easily could an exploit be constructed based on the patch? Yes. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? Yes. The bug is trivial to see. Which older supported branches are affected by this flaw? All of them. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? It's easy to backport. How likely is this patch to cause regressions; how much testing does it need? none.

Attachment #8613543 - Flags: sec-approval?

Comment 7

•

9 years ago

sec-approval+ for trunk. We'll want this on: Aurora Beta ESR38 ESR31 as well. If you could create and nominate patches after it lands on Trunk, it would be appreciated.

status-firefox38: --- → wontfix

status-firefox38.0.5: --- → wontfix

status-firefox39: --- → affected

status-firefox40: --- → affected

status-firefox-esr31: --- → affected

status-firefox-esr38: --- → affected

tracking-firefox39: ? → +

tracking-firefox40: ? → +

tracking-firefox41: ? → +

tracking-firefox-esr31: ? → 39+

tracking-firefox-esr38: ? → 39+

Updated

•

9 years ago

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

Assignee

Comment 8

•

9 years ago

Comment on attachment 8613543 [details] [diff] [review] crash2.patch [Approval Request Comment] User impact if declined: FF crashes. 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.

Attachment #8613543 - Flags: approval-mozilla-esr38?

Attachment #8613543 - Flags: approval-mozilla-esr31?

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

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

Assignee

Updated

•

9 years ago

Keywords: checkin-needed

Updated

•

9 years ago

Attachment #8613543 - Flags: approval-mozilla-esr38?

Attachment #8613543 - Flags: approval-mozilla-esr38+

Attachment #8613543 - Flags: approval-mozilla-esr31?

Attachment #8613543 - Flags: approval-mozilla-esr31+

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

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

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

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

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

Assignee

Comment 9

•

9 years ago

Keywords: checkin-needed

https://hg.mozilla.org/releases/mozilla-aurora/rev/9864769ebb1e

Comment 10

•

9 years ago

status-b2g-v2.0: --- → affected

status-b2g-v2.0M: --- → affected

status-b2g-v2.1: --- → affected

status-b2g-v2.1S: --- → affected

status-b2g-v2.2: --- → affected

status-b2g-master: --- → fixed

status-firefox40: affected → fixed

Target Milestone: --- → mozilla41

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

Comment 11

•

9 years ago

status-firefox39: affected → fixed

Flags: in-testsuite?

https://hg.mozilla.org/releases/mozilla-esr38/rev/6164a0d6b1d4 https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/47d9d30f6811 https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/878f373ff531 https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/878f373ff531 https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/37d08af8473e https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/37d08af8473e https://hg.mozilla.org/releases/mozilla-esr31/rev/b64bd9720367

Comment 12

•

9 years ago

status-b2g-v2.0: affected → fixed

status-b2g-v2.0M: affected → fixed

status-b2g-v2.1: affected → fixed

status-b2g-v2.1S: affected → fixed

status-b2g-v2.2: affected → fixed

status-firefox-esr31: affected → fixed

status-firefox-esr38: affected → fixed

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

Comment 13

•

9 years ago

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

Status: NEW → RESOLVED

Closed: 9 years ago

Resolution: --- → FIXED

Reporter

Comment 14

•

9 years ago

When I first saw the patch crash2.patch, I was thinking it probably did not fix the problem at all. Now the patch is in trunk and I can verify. Run the same test case (Uaf_CanonicalizeXPCOMParticipant_DedicatedWorker_repro.js) with latest code, I got: First-chance exception at 0x65707974 in firefox.exe: 0xC0000005: Access violation executing location 0x65707974. Unhandled exception at 0x65707974 in firefox.exe: 0xC0000005: Access violation executing location 0x65707974. Firefox version: 41.0a1 (2015-06-05) Call Stack: 65707974() Unknown [Frames below may be incorrect and/or missing] > xul.dll!CanonicalizeXPCOMParticipant(nsISupports * aIn) Line 931 C++ xul.dll!CCGraphBuilder::NoteXPCOMChild(nsISupports * aChild) Line 2331 C++ xul.dll!mozilla::CycleCollectedJSRuntime::NoteGCThingXPCOMChildren(const js::Class * aClasp, JSObject * aObj, nsCycleCollectionTraversalCallback & aCb) Line 605 C++ xul.dll!mozilla::CycleCollectedJSRuntime::TraverseGCThing(mozilla::CycleCollectedJSRuntime::TraverseSelect aTs, JS::GCCellPtr aThing, nsCycleCollectionTraversalCallback & aCb) Line 638 C++ xul.dll!mozilla::JSGCThingParticipant::Traverse(void * aPtr, nsCycleCollectionTraversalCallback & aCb) Line 335 C++ xul.dll!CCGraphBuilder::BuildGraph(js::SliceBudget & aBudget) Line 2239 C++ xul.dll!nsCycleCollector::MarkRoots(js::SliceBudget & aBudget) Line 2840 C++ xul.dll!nsCycleCollector::Collect(ccType aCCType, js::SliceBudget & aBudget, nsICycleCollectorListener * aManualListener, bool aPreferShorterSlices) Line 3611 C++ xul.dll!nsCycleCollector_collect(nsICycleCollectorListener * aManualListener) Line 4098 C++ xul.dll!`anonymous namespace'::WorkerJSRuntime::CustomGCCallback(JSGCStatus aStatus) Line 998 C++ xul.dll!mozilla::CycleCollectedJSRuntime::OnGC(JSGCStatus aStatus) Line 1240 C++ xul.dll!mozilla::CycleCollectedJSRuntime::GCCallback(JSRuntime * aRuntime, JSGCStatus aStatus, void * aData) Line 732 C++ xul.dll!js::gc::GCRuntime::collect(bool incremental, js::SliceBudget budget, JS::gcreason::Reason reason) Line 6147 C++ xul.dll!js::gc::GCRuntime::gc(JSGCInvocationKind gckind, JS::gcreason::Reason reason) Line 6204 C++ xul.dll!js::DestroyContext(JSContext * cx, js::DestroyContextMode mode) Line 187 C++ xul.dll!JS_DestroyContext(JSContext * cx) Line 738 C++ xul.dll!`anonymous namespace'::WorkerThreadPrimaryRunnable::Run() Line 2800 C++ xul.dll!nsThread::ProcessNextEvent(bool aMayWait, bool * aResult) Line 846 C++ xul.dll!NS_ProcessNextEvent(nsIThread * aThread, bool aMayWait) Line 265 C++ xul.dll!mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate * aDelegate) Line 355 C++ xul.dll!MessageLoop::RunHandler() Line 227 C++ xul.dll!MessageLoop::Run() Line 201 C++ xul.dll!nsThread::ThreadFunc(void * aArg) Line 361 C++ nss3.dll!_PR_NativeRunThread(void * arg) Line 419 C nss3.dll!pr_root(void * arg) Line 90 C [External Code] Variables: - aIn 0x0f44e160 {...} nsISupports * + __vfptr xul.dll!0x5a5a5a5a {0x65707974, DWrite.dll!0x6f5b0000, 0x63656a62} void * * + out 0x00000000 <NULL> nsISupports * Registers: EAX = 0F44E160 EBX = 00000000 ECX = 5A5A5A5A EDX = 16DCF8F0 ESI = 5A2D41AC EDI = 0FE38940 EIP = 58954E86 ESP = 16DCF8E4 EBP = 00000000 EFL = 00000000 0xfffffffc = 00000000

Updated

•

9 years ago

Status: RESOLVED → REOPENED

Resolution: FIXED → ---

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

Updated

•

9 years ago

Flags: needinfo?(amarchesini)

Comment 15

•

9 years ago

Can you also reproduce this with 40 and 39?

Flags: needinfo?(loobenyang)

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

Reporter

Comment 16

•

9 years ago

(In reply to Liz Henry (:lizzard) from comment #15) > Can you also reproduce this with 40 and 39? Yes. Just checked out the beta code and ran it in Visual Studio with the same test case (Uaf_CanonicalizeXPCOMParticipant_DedicatedWorker_repro.js), got: First-chance exception at 0x50F82E67 (xul.dll) in firefox.exe: 0xC0000005: Access violation reading location 0x5A5A5A5A. Unhandled exception at 0x50F82E67 (xul.dll) in firefox.exe: 0xC0000005: Access violation reading location 0x5A5A5A5A. Firefox version: 39.0 Variables: - aIn 0x14c14020 {...} nsISupports * + __vfptr 0x5a5a5a5a {???, ???, ???} void * * + out 0x00000000 <NULL> nsISupports * Stack: > xul.dll!CanonicalizeXPCOMParticipant(nsISupports * aIn) Line 931 C++ xul.dll!CCGraphBuilder::NoteXPCOMChild(nsISupports * aChild) Line 2356 C++ xul.dll!mozilla::CycleCollectedJSRuntime::NoteGCThingXPCOMChildren(const js::Class * aClasp, JSObject * aObj, nsCycleCollectionTraversalCallback & aCb) Line 630 C++ xul.dll!mozilla::CycleCollectedJSRuntime::TraverseGCThing(mozilla::CycleCollectedJSRuntime::TraverseSelect aTs, JS::GCCellPtr aThing, nsCycleCollectionTraversalCallback & aCb) Line 663 C++ xul.dll!mozilla::JSGCThingParticipant::Traverse(void * aPtr, nsCycleCollectionTraversalCallback & aCb) Line 352 C++ xul.dll!CCGraphBuilder::BuildGraph(js::SliceBudget & aBudget) Line 2259 C++ xul.dll!nsCycleCollector::MarkRoots(js::SliceBudget & aBudget) Line 2865 C++ xul.dll!nsCycleCollector::Collect(ccType aCCType, js::SliceBudget & aBudget, nsICycleCollectorListener * aManualListener, bool aPreferShorterSlices) Line 3617 C++ xul.dll!nsCycleCollector_collect(nsICycleCollectorListener * aManualListener) Line 4090 C++ xul.dll!`anonymous namespace'::WorkerJSRuntime::CustomGCCallback(JSGCStatus aStatus) Line 971 C++ xul.dll!mozilla::CycleCollectedJSRuntime::OnGC(JSGCStatus aStatus) Line 1244 C++ xul.dll!mozilla::CycleCollectedJSRuntime::GCCallback(JSRuntime * aRuntime, JSGCStatus aStatus, void * aData) Line 757 C++ xul.dll!js::gc::GCRuntime::collect(bool incremental, js::SliceBudget budget, JS::gcreason::Reason reason) Line 6113 C++ xul.dll!js::gc::GCRuntime::gc(JSGCInvocationKind gckind, JS::gcreason::Reason reason) Line 6170 C++ xul.dll!js::DestroyContext(JSContext * cx, js::DestroyContextMode mode) Line 188 C++ xul.dll!JS_DestroyContext(JSContext * cx) Line 681 C++ xul.dll!`anonymous namespace'::WorkerThreadPrimaryRunnable::Run() Line 2746 C++ xul.dll!nsThread::ProcessNextEvent(bool aMayWait, bool * aResult) Line 855 C++ xul.dll!NS_ProcessNextEvent(nsIThread * aThread, bool aMayWait) Line 265 C++ xul.dll!mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate * aDelegate) Line 368 C++ xul.dll!MessageLoop::RunHandler() Line 227 C++ xul.dll!MessageLoop::Run() Line 201 C++ xul.dll!nsThread::ThreadFunc(void * aArg) Line 358 C++ nss3.dll!_PR_NativeRunThread(void * arg) Line 419 C nss3.dll!pr_root(void * arg) Line 90 C [External Code] [Frames below may be incorrect and/or missing, no symbols loaded for msvcr120.dll]

Flags: needinfo?(loobenyang)

Comment 17

•

9 years ago

OK. Thanks Looben. Looks like we need to mark this as still not fixed for 39 and 40 then.

status-firefox39: fixed → affected

status-firefox40: fixed → affected

[SV Manager] Florin Mezei, QA (:FlorinMezei)

Comment 18

•

9 years ago

Should this be verified also on ESR 31 and 38, or is it safe to assume those would be affected as well?

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

Comment 19

•

9 years ago

I think we can assume they're affected, yes.

Ben Turner (not reading bugmail, use the needinfo flag!)

Assignee

Comment 20

•

9 years ago

Attached patch crash2.patch (obsolete) — Details — Splinter Review

Flags: needinfo?(amarchesini)

Attachment #8617392 - Flags: review?(bent.mozilla)

Comment 21

•

9 years ago

Comment on attachment 8617392 [details] [diff] [review] crash2.patch Review of attachment 8617392 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/XMLHttpRequest.cpp @@ +1877,5 @@ > mProxy->mOuterChannelId++; > > JSContext* cx = mWorkerPrivate->GetJSContext(); > > + nsRefPtr<XMLHttpRequest> kungFuDeathGrip(this); This really shouldn't be needed because the binding object should be holding this object alive. We need to figure out why that isn't happening. I suggest inspecting the binding object when we crash. It should be on the stack. And see if the binding object still has a pointer to the c++ object in its private slot.

Attachment #8617392 - Flags: review?(bent.mozilla) → review-

Assignee

Comment 22

•

9 years ago

Attached patch crash2.patch (obsolete) — Details — Splinter Review

You are right, I don't have to keep the obj alive.

Attachment #8613543 - Attachment is obsolete: true

Attachment #8617392 - Attachment is obsolete: true

Attachment #8617836 - Flags: review?(bent.mozilla)

Ben Turner (not reading bugmail, use the needinfo flag!)

Updated

•

9 years ago

status-b2g-v2.0: fixed → affected

status-b2g-v2.0M: fixed → affected

status-b2g-v2.1: fixed → affected

status-b2g-v2.1S: fixed → affected

status-b2g-v2.2: fixed → affected

status-b2g-master: fixed → affected

status-firefox-esr31: fixed → affected

status-firefox-esr38: fixed → affected

Target Milestone: mozilla41 → ---

Comment 23

•

9 years ago

Comment on attachment 8617836 [details] [diff] [review] crash2.patch Review of attachment 8617836 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/XMLHttpRequest.cpp @@ +1882,5 @@ > new SendRunnable(mWorkerPrivate, mProxy, aStringBody, Move(aBody), > aClonedObjects, syncLoopTarget, hasUploadListeners); > if (!runnable->Dispatch(cx)) { > + // If we are not rooted now, it seems that the worker thread is going away > + // soon and we have been unpinned by the feature. Maybe say: Dispatch() may have spun the event loop and we may have already unrooted. If so we don't want autoUnpin to try again.

Attachment #8617836 - Flags: review?(bent.mozilla) → review+

Assignee

Comment 24

•

9 years ago

Attached patch crash2.patch — Details — Splinter Review

Attachment #8617836 - Attachment is obsolete: true

Assignee

Comment 25

•

9 years ago

Comment on attachment 8620914 [details] [diff] [review] crash2.patch Read comment 6.

Attachment #8620914 - Flags: sec-approval?

Comment 26

•

9 years ago

Comment on attachment 8620914 [details] [diff] [review] crash2.patch sec-approval+ for trunk. We'll want patches for all affected branches as well.

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

Assignee

Updated

•

9 years ago

Keywords: checkin-needed

https://hg.mozilla.org/integration/mozilla-inbound/rev/281a5a64be5e

Comment 27

•

9 years ago

Keywords: checkin-needed

https://hg.mozilla.org/releases/mozilla-aurora/rev/1b69a66f8756 https://hg.mozilla.org/releases/mozilla-beta/rev/aaf1311249a8

Comment 28

•

9 years ago

status-firefox39: affected → fixed

status-firefox40: affected → fixed

https://hg.mozilla.org/mozilla-central/rev/281a5a64be5e

Comment 29

•

9 years ago

Status: REOPENED → RESOLVED

Closed: 9 years ago → 9 years ago

status-b2g-master: affected → fixed

status-firefox41: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla41

Reporter

Comment 30

•

9 years ago

Thanks Andrea for the prompt fix! - Looben

https://hg.mozilla.org/releases/mozilla-esr38/rev/28b3bd9de270 https://hg.mozilla.org/releases/mozilla-esr31/rev/4028bc28f4c6

Comment 31

•

9 years ago

status-firefox-esr31: affected → fixed

status-firefox-esr38: affected → fixed

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/ffee57f621f1 https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/cd6e051445c0 https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/cd6e051445c0 https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/d78115b1ed80 https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/d78115b1ed80

Comment 32

•

9 years ago

status-b2g-v2.0: affected → fixed

status-b2g-v2.0M: affected → fixed

status-b2g-v2.1: affected → fixed

status-b2g-v2.1S: affected → fixed

status-b2g-v2.2: affected → fixed

Comment 33

•

9 years ago

Looben, can you verify that this fixed across the board now?

Flags: sec-bounty?

Flags: sec-bounty+

Flags: needinfo?(loobenyang)

Reporter

Comment 34

•

9 years ago

(In reply to Al Billings [:abillings] from comment #33) > Looben, can you verify that this fixed across the board now? I've already verified it's fixed in trunk and latest official linux asan build. Thanks for the quick turnaround.

Flags: needinfo?(loobenyang)

Updated

•

9 years ago

Whiteboard: [adv-main39+][adv-esr38.1+][adv-esr31.8+]