Closed Bug 808736 Opened 7 years ago Closed 7 years ago

sJSGCThingRootCount maybe can in theory be abused to trigger nsLayoutStatics::Shutdown()

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox17 --- wontfix
firefox18 --- affected
firefox19 --- affected
firefox20 --- fixed
firefox-esr10 --- wontfix
firefox-esr17 --- wontfix
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- fixed

People

(Reporter: mccr8, Unassigned)

References

Details

(Keywords: sec-low, Whiteboard: [fixed by bug 811206][adv-main20+])

Whenever we call nsContentUtils::HoldJSObjects, we increment sJSGCThingRootCount, and call nsLayoutStatics::AddRef() if the count was initially 0. Whenever we call nsContentUtils::DropJSObjects, we decrement the count, and call nsLayoutStatics::Release() if the count ends up at 0.

If the static release hits 0, then a bunch of shutdown stuff gets triggered.

The problem here is that there's no checking that the number of calls to HOLD is actually balanced with the number of DROP calls. I noticed that with AudioContext (and probably any other wrapper cached object that also have JS objects) we can end up calling DROP multiple times for a single object, once if the object is preserved, for the wrapper, and once for the extra JS stuff I guess.

Thus, by creating and destroying an AudioContext, we can cause sJSGCThingRootCount to decrease. If we do this enough, eventually the count will be 0, and we'll call nsLayoutStatics::Release(). This is probably not a huge deal is isolation, because at worst it will slightly mess up shutdown. But we can try to do this repeatedly to cause it to happen otherwise.

Note that we have to do this without calling nsContentUtils::HoldJSObjects when the refcount is 0, because that will just cause nsLayoutStatics::AddRef() to trigger again, thwarting us.

But, once it is dropping past zero, we can just keep going, then the count will underflow and up at a large value. Thus for every 4.3 billion AudioContexts we create and destroy, we can cause the static count to drop. They aren't alive at the same time (though you'd want to have a bunch alive at the same time to reduce the chance we call HOLD when the count is exactly zero).

This doesn't seem like a practical attack to me, but I have see the count underflow, so there's some kind of issue here we should fix.
I'm going to mark this as sec-low, because even if we can generate and destroy 100,000 audiocontexts a second, it would still take 12 hours to trigger the second nsLayoutStatics::Release()...
Keywords: sec-low
Depends on: 811206
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
Whiteboard: [fixed by bug 811206]
Whiteboard: [fixed by bug 811206] → [fixed by bug 811206][adv-main20+]
Group: core-security
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.