Closed Bug 852817 Opened 7 years ago Closed 7 years ago

"ASSERTION: JS binding already finalized" with AudioContext.destination

Categories

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

x86_64
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla22
Tracking Status
firefox21 --- unaffected
firefox22 --- fixed
firefox-esr17 --- unaffected
b2g18 --- unaffected

People

(Reporter: jruderman, Assigned: ehsan)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(2 files)

1. Grab build https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-macosx64-debug/1363725913/
2. Install extension https://www.squarefree.com/extensions/domFuzzLite3.xpi
3. Load the testcase
4. Quit Firefox

Result: during a shutdown GC,

###!!! ASSERTION: JS binding already finalized: '!mJSBindingFinalized', file AudioNode.h, line 77

Regression in:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=09f72f45a0b7&tochange=126563fd3ba1
mJSBindingFinalized needs to be set to false when an AudioNode gets wrapped, there can be multiple calls to the JSBindingFinalized.
Is this actually a security issue?  It looks like after this assertion in JSBindingFinalized() we call UpdateOutputEnded(), which returns if mOutputEnded is true, then sets mOutputEnded to true, so it doesn't look like we'd actually do anything in this case.
If it is, I assume it would be sec-crit?
Yeah this is not a security issue.
Blocks: webaudio
Group: core-security
Attached patch Patch (v1)Splinter Review
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #727474 - Flags: review?(roc)
Doesn't this mean we have a bug where someone adds expando properties to a DestinationNode and then the wrapper can be GCed, then we get the destination node from the AudioContext again and the expandos have mysteriously disappeared?
That sort of problem is really hard to produce with the WebIDL bindings.  ;)

In particular, DestinationNode is not marked as "'wrapperCache': False", so if an expando is added we'll preserve its wrapper (by creating a cycle between the JS wrapper and the C++ object so they have the same lifetime after that point).

And it can't be marked as "'wrapperCache': False", since it's the return value of an IDL member not marked [Creator], and codegen checks for something non-wrappercached being returned from a non-creator and throws an exception.

Basically, the only way to screw this up is to mark a class as not wrappercached _and_ either mis-mark something as [Creator] or return the object in question via an "object" or "any" (or perhaps ancestor interface; we may not do a good job of checking for that, actually).
Comment on attachment 727474 [details] [diff] [review]
Patch (v1)

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

great!
Attachment #727474 - Flags: review?(roc) → review+
https://hg.mozilla.org/mozilla-central/rev/5e49ff80f015
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.