Closed Bug 1142210 (CVE-2015-2728) Opened 5 years ago Closed 5 years ago

Type Confusion mozilla::dom::indexedDB::IndexedDatabaseManager::CommonPostHandleEvent


(Core :: Storage: IndexedDB, defect)

Not set



Tracking Status
firefox37 --- wontfix
firefox38 --- wontfix
firefox38.0.5 --- wontfix
firefox39 + verified
firefox-esr31 39+ verified
firefox-esr38 39+ verified
b2g-v2.0 --- fixed
b2g-v2.0M --- fixed
b2g-v2.1 --- fixed
b2g-v2.1S --- fixed
b2g-v2.2 --- fixed
b2g-master --- fixed


(Reporter: bandha890, Assigned: bent.mozilla)



(Keywords: qawanted, regression, sec-high, Whiteboard: [adv-main39+][adv-esr38.1+][adv-esr31.8+])


(3 files)

Attached file testevent2.html
User Agent: Opera/9.80 (X11; Linux x86_64) Presto/2.12.388 Version/12.16

Steps to reproduce:

Run file

Actual results:

I looked at it on debug build it seems to cast an IDBDatabase object to IDBRequest, then checks IDBRequest::mOwningThread equals current thread, but actually reads from database object. That's why I put type confusion, only had a quick look though.

#0  mozilla::dom::indexedDB::IDBRequest::AssertIsOnOwningThread (this=<optimized out>)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/dom/indexedDB/IDBRequest.cpp:72
#1  0x00007fffe8866bce in mozilla::dom::indexedDB::IDBRequest::GetErrorAfterResult (this=0x612000528f40)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/dom/indexedDB/IDBRequest.cpp:259
#2  0x00007fffe887da17 in mozilla::dom::indexedDB::IndexedDatabaseManager::CommonPostHandleEvent (aEventTarget=<optimized out>, aFactory=<optimized out>, 
    aVisitor=...) at /builds/slave/m-beta-l64-asan-d-000000000000/build/dom/indexedDB/IndexedDatabaseManager.cpp:406
#3  0x00007fffe88475e4 in mozilla::dom::indexedDB::IDBDatabase::PostHandleEvent (this=0x612000528f40, aVisitor=...)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/dom/indexedDB/IDBDatabase.cpp:1339
#4  0x00007fffe7fefd0a in mozilla::EventTargetChainItem::HandleEventTargetChain (aChain=..., aVisitor=..., aCallback=<optimized out>, aCd=...)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/dom/events/EventDispatcher.cpp:302
#5  0x00007fffe7ff01fc in mozilla::EventTargetChainItem::HandleEventTargetChain (aChain=..., aVisitor=..., aCallback=<optimized out>, aCd=...)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/dom/events/EventDispatcher.cpp:348
#6  0x00007fffe7ff1d54 in mozilla::EventDispatcher::Dispatch (aTarget=<optimized out>, aPresContext=<optimized out>, aEvent=<optimized out>, 
    aDOMEvent=<optimized out>, aEventStatus=<optimized out>, aCallback=<optimized out>, aTargets=<optimized out>)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/dom/events/EventDispatcher.cpp:633
#7  0x00007fffe7fcd5dc in mozilla::EventDispatcher::DispatchDOMEvent (aTarget=<optimized out>, aEvent=<optimized out>, aDOMEvent=0x608000382e20, 
    aPresContext=<optimized out>, aEventStatus=<optimized out>) at /builds/slave/m-beta-l64-asan-d-000000000000/build/dom/events/EventDispatcher.cpp:697
#8  0x00007fffe7fdc9da in mozilla::DOMEventTargetHelper::DispatchEvent (this=0x612000528f40, aEvent=0x608000382e20, aRetVal=0x7fffffff6540)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/dom/events/DOMEventTargetHelper.cpp:248
#9  0x00007fffe6df1a3b in mozilla::dom::EventTarget::DispatchEvent (this=0x612000528f40, aEvent=..., aRv=...)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/dom/base/nsINode.cpp:2722
#10 0x00007fffe72ce35c in mozilla::dom::EventTargetBinding::dispatchEvent (cx=<optimized out>, self=<optimized out>, args=..., obj=...)
    at ./EventTargetBinding.cpp:164
#11 0x00007fffe72cddec in mozilla::dom::EventTargetBinding::genericMethod (cx=<optimized out>, argc=<optimized out>, vp=<optimized out>)
    at ./EventTargetBinding.cpp:342
#12 0x00007fffeba9b526 in js::CallJSNative (cx=<optimized out>, native=<optimized out>, args=...)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/js/src/jscntxtinlines.h:227
#13 0x00007fffeba66083 in js::Invoke (cx=<optimized out>, args=..., construct=<optimized out>)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/js/src/vm/Interpreter.cpp:498
#14 0x00007fffeba9101b in Interpret (cx=0x614000252240, state=...) at /builds/slave/m-beta-l64-asan-d-000000000000/build/js/src/vm/Interpreter.cpp:2556
#15 0x00007fffeba8256c in js::RunScript (cx=<optimized out>, state=...) at /builds/slave/m-beta-l64-asan-d-000000000000/build/js/src/vm/Interpreter.cpp:448
#16 0x00007fffeba661fe in js::Invoke (cx=<optimized out>, args=..., construct=<optimized out>)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/js/src/vm/Interpreter.cpp:517
#17 0x00007fffeba9bac5 in js::Invoke (cx=0x614000252240, thisv=..., fval=..., argc=<optimized out>, argv=<optimized out>, rval=...)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/js/src/vm/Interpreter.cpp:554
#18 0x00007fffeb785c92 in JS::Call (cx=0x614000252240, args=..., thisv=..., fval=..., rval=...)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/js/src/jsapi.cpp:4564
#19 0x00007fffe735e40f in mozilla::dom::Function::Call (this=<optimized out>, cx=0x614000252240, arguments=..., aRv=..., aThisVal=..., aRetVal=...)
    at ./FunctionBinding.cpp:35
#20 0x00007fffe6b4e6bf in mozilla::dom::Function::Call<nsCOMPtr<nsISupports> > (this=<optimized out>, thisObjPtr=..., arguments=..., aRv=..., 
    aExceptionHandling=<optimized out>, aRetVal=...) at ../../dist/include/mozilla/dom/FunctionBinding.h:58
#21 0x00007fffe6b4dcbb in nsGlobalWindow::RunTimeoutHandler (this=<optimized out>, aTimeout=<optimized out>, aScx=<optimized out>)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/dom/base/nsGlobalWindow.cpp:12261
#22 0x00007fffe6b35034 in nsGlobalWindow::RunTimeout (this=<optimized out>, aTimeout=<optimized out>)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/dom/base/nsGlobalWindow.cpp:12485
#23 0x00007fffe6b4d24a in nsGlobalWindow::TimerCallback (aClosure=<optimized out>, aTimer=<optimized out>)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/dom/base/nsGlobalWindow.cpp:12732
#24 0x00007fffe534472a in nsTimerImpl::Fire (this=<optimized out>) at /builds/slave/m-beta-l64-asan-d-000000000000/build/xpcom/threads/nsTimerImpl.cpp:631
#25 0x00007fffe53450cf in nsTimerEvent::Run (this=<optimized out>) at /builds/slave/m-beta-l64-asan-d-000000000000/build/xpcom/threads/nsTimerImpl.cpp:724
#26 0x00007fffe533c16b in nsThread::ProcessNextEvent (this=<optimized out>, aMayWait=<optimized out>, aResult=<optimized out>)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/xpcom/threads/nsThread.cpp:855
#27 0x00007fffe539dbcf in NS_ProcessNextEvent (aThread=<optimized out>, aMayWait=false)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/xpcom/glue/nsThreadUtils.cpp:265
#28 0x00007fffe5af76fe in mozilla::ipc::MessagePump::Run (this=<optimized out>, aDelegate=<optimized out>)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/ipc/glue/MessagePump.cpp:99
#29 0x00007fffe5a948e2 in MessageLoop::RunInternal (this=<optimized out>)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/ipc/chromium/src/base/
#30 0x00007fffe5a94789 in MessageLoop::Run (this=0x61400002ee40)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/ipc/chromium/src/base/
#31 0x00007fffe8c9c107 in nsBaseAppShell::Run (this=<optimized out>) at /builds/slave/m-beta-l64-asan-d-000000000000/build/widget/nsBaseAppShell.cpp:164
#32 0x00007fffea00f0c6 in nsAppStartup::Run (this=0x6070000269e0)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/toolkit/components/startup/nsAppStartup.cpp:281
#33 0x00007fffea0cbbbd in XREMain::XRE_mainRun (this=<optimized out>) at /builds/slave/m-beta-l64-asan-d-000000000000/build/toolkit/xre/nsAppRunner.cpp:4141
#34 0x00007fffea0ccd41 in XREMain::XRE_main (this=0x7fffffffc580, argc=<optimized out>, argv=<optimized out>, aAppData=<optimized out>)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/toolkit/xre/nsAppRunner.cpp:4217
#35 0x00007fffea0cd7d3 in XRE_main (argc=1, argv=0x7fffffffde58, aAppData=<optimized out>, aFlags=<optimized out>)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/toolkit/xre/nsAppRunner.cpp:4437
#36 0x000000000048c251 in do_main (argc=<optimized out>, argv=<optimized out>, xreDirectory=<optimized out>)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/browser/app/nsBrowserApp.cpp:292
#37 0x000000000048b7d2 in main (argc=1, argv=0x7ffff6f91a20 <_IO_stdfile_2_lock>)
    at /builds/slave/m-beta-l64-asan-d-000000000000/build/browser/app/nsBrowserApp.cpp:661

Expected results:

Component: Untriaged → DOM: IndexedDB
Product: Firefox → Core
Could you look at this, Kyle?  Thanks.
Flags: needinfo?(khuey)
Flags: needinfo?(khuey) → needinfo?(bent.mozilla)
I'm not able to reproduce this with the debug version of Firefox 39.0a1 2015-03-17 using the information provided.
QA Whiteboard: [triaged]
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #3)
> I'm not able to reproduce this with the debug version of Firefox 39.0a1
> 2015-03-17 using the information provided.

That's ok, it's an obvious bug. I will fix it.
Assignee: nobody → bent.mozilla
Ever confirmed: true
Flags: needinfo?(bent.mozilla)
Flags: sec-bounty?
Attached patch Patch, v1Splinter Review
This fixes it two ways :)
Attachment #8579761 - Flags: review?(khuey)
What security rating do you think this should get?  Type confusion where we're calling methods on an object of the wrong class seems like a sec-high to me.  What branches does this affect?
Flags: needinfo?(bent.mozilla)
I think that's probably true. There doesn't seem to be any way to get this code to touch garbage memory (all the objects involved are live and valid), but it looks like the code will grab a piece of an IDBDatabase (part of a hashtable) and use it as a DOMError pointer. Then it will interpret some offset of that pointer as a nsString, and that has a mData pointer that could point wherever, and it will probably pretend that there's an nsStringBuffer there and increment/decrement a refcount, so it will probably subtly corrupt memory if it doesn't crash...

Basically just a big mess, but I don't think you could get this to execute arbitrary code, at least not from a quick first pass look. If it's vital we know I can dig deeper.
Flags: needinfo?(bent.mozilla)
(In reply to Ben Turner [:bent] (use the needinfo flag!) from comment #8)
> Basically just a big mess, but I don't think you could get this to execute
> arbitrary code, at least not from a quick first pass look. If it's vital we
> know I can dig deeper.

Alright.  As a sec-high that I assume affects more than trunk, this should have gotten sec-approval before landing.  If you could fill that out, I'd appreciate it.  If it only affects trunk, then you don't need to bother.
Flags: needinfo?(bent.mozilla)
Keywords: sec-high
Flags: needinfo?(bent.mozilla)
Comment on attachment 8579761 [details] [diff] [review]
Patch, v1

[Security approval request comment]
How easily could an exploit be constructed based on the patch? The patch is pretty broad, but it's reasonably easy to see what the problem is.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No tests included, but see previous answer.

Which older supported branches are affected by this flaw? All the way back to FF25

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

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

How likely is this patch to cause regressions; how much testing does it need? It's very safe.
Attachment #8579761 - Flags: sec-approval?
I think this missed the last beta for 37. I want to get Lawrence's opinion about taking this.
Flags: needinfo?(lmandel)
This did miss the last Beta. This wasn't even on my list as it was only recently flagged as a sec-high and is not tracked. Unless we think this must ship in 37, I would prefer to ship this fix in 38.
Flags: needinfo?(lmandel)
Ok. Since comment 12 says this would be pretty easy to spot once it is in, I'm going to give sec-approval+ for checkin on April 14, two weeks into the next cycle.
Whiteboard: [checkin on 4/14]
Attachment #8579761 - Flags: sec-approval? → sec-approval+
Al, you saw that I already checked it in? Sorry :(

No, I missed that. Let me talk to release management.
Whiteboard: [checkin on 4/14]
Let's get this onto Aurora, at least.
Keywords: regression
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
(In reply to Phil Ringnalda (:philor) from comment #19)

Paul, can you please confirm that this is fixed for you?
Flags: needinfo?(bandha890)
Yep it's fixed for me.
Flags: needinfo?(bandha890)
(In reply to Paul from comment #21)
> Yep it's fixed for me.

Thanks for your help! I'm marking this verified fixed since it's been verified on Nightly. We'll probably need your help to verify this once it gets uplifted to Aurora 38 and ESR 31.
Flags: sec-bounty? → sec-bounty+
This never landed on 38? Seems we need approval requests for at least esr31/esr38 ASAP. However, Ben and Kyle are both away AFAIK, so I'm not sure who can do this request.
Comment on attachment 8579761 [details] [diff] [review]
Patch, v1

a=dveditz for landing on the ESR branches.
Attachment #8579761 - Flags: approval-mozilla-esr38+
Attachment #8579761 - Flags: approval-mozilla-esr31+
Flags: needinfo?(abillings)
This is hitting conflicts on Gecko <37 due to bug 701634. It'll need rebasing.
Flags: needinfo?(bent.mozilla)
Attached patch Patch for esr31Splinter Review
Flags: needinfo?(bent.mozilla)
Can someone please verify this on esr31?
Keywords: qawanted
Whiteboard: [adv-main39+][adv-esr38.1+][adv-esr31.8+]
Alias: CVE-2015-2728
For what it's worth, broke unified builds on esr31 (which I had been using on Debian).
I am unable to reproduce the initial issue as well using an old debug build from 2015-03-11, so will not be able to verify the fix.
Paul can you please verify that this is fixed on ESR builds as well?

Firefox 38.1.0ESR

Firefox 31.8.0ESR
Flags: needinfo?(bandha890)
Yes they're fixed for me.
Flags: needinfo?(bandha890)
(In reply to Paul from comment #33)
> Yes they're fixed for me.

Thanks Paul! Marking as verified on ESR builds.
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.