Closed Bug 872812 Opened 6 years ago Closed 6 years ago

Passing viewBox to init*Event causes an assertion failure in CC macro expansion

Categories

(Core :: SVG, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla24
Tracking Status
firefox22 --- unaffected
firefox23 + fixed
firefox24 + fixed
firefox-esr17 --- unaffected
b2g18 --- unaffected

People

(Reporter: jruderman, Assigned: baku)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [adv-main23-])

Attachments

(3 files, 1 obsolete file)

Assertion failure: !aIID.Equals((::nsISupports::COMTypeInfo<int>::kIID)), at content/svg/content/src/SVGAnimatedRect.cpp:18
Attached file stack (gdb)
Looks like sec-critical. viewBox handling looks very odd.
Keywords: sec-critical
(but I assume this is a regression from webidlification.)
The code is:

15 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(SVGAnimatedRect)
16   NS_INTERFACE_MAP_ENTRY(nsISupports)
17   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
18 NS_INTERFACE_MAP_END

NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY strikes again.  See bug 798188.

The order of lines 16 and 17 needs to be switched.
Blocks: 866796
Assignee: nobody → continuation
Attached patch patch (obsolete) — Splinter Review
Still testing it...
Attachment #750571 - Flags: review?(bzbarsky)
Comment on attachment 750571 [details] [diff] [review]
patch

r=me, but you need to hg add the test file.
Attachment #750571 - Flags: review?(bzbarsky) → review+
Attached patch patchSplinter Review
Attachment #750571 - Attachment is obsolete: true
Attachment #750598 - Flags: review+
Keywords: checkin-needed
This can't land without security approval.
Keywords: checkin-needed
Boris, can you help me with this? I just changed the order of 2 lines :)
Flags: needinfo?(bzbarsky)
You just need to request security approval on the patch.
Flags: needinfo?(bzbarsky)
Comment on attachment 750598 [details] [diff] [review]
patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch? I have no idea
   how to do it.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?  The tests show how to trigger the
   assert, but I'm not sure how to exploit it.

Which older supported branches are affected by this flaw?  Aurora 23.

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

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?  This should apply to Aurora 23.

How likely is this patch to cause regressions; how much testing does it need? 
   This is a very safe patch.
Attachment #750598 - Flags: sec-approval?
Thanks!
Assignee: continuation → amarchesini
Comment on attachment 750598 [details] [diff] [review]
patch

sec-approval+ for m-c. Please prepare an aurora patch and nominate once it is on trunk.
Attachment #750598 - Flags: sec-approval? → sec-approval+
Comment on attachment 750598 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 866796
User impact if declined: a crash...
Risk to taking this patch (and alternatives if risky): nothing.
Attachment #750598 - Flags: approval-mozilla-aurora?
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f74414ca211d
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Attachment #750598 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [adv-main23-]
Group: core-security
You need to log in before you can comment on or make changes to this bug.