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

RESOLVED FIXED in Firefox 23

Status

()

Core
SVG
--
critical
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Jesse Ruderman, Assigned: baku)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla24
x86_64
Mac OS X
assertion, regression, sec-critical, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox22 unaffected, firefox23+ fixed, firefox24+ fixed, firefox-esr17 unaffected, b2g18 unaffected)

Details

(Whiteboard: [adv-main23-])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Assertion failure: !aIID.Equals((::nsISupports::COMTypeInfo<int>::kIID)), at content/svg/content/src/SVGAnimatedRect.cpp:18
(Reporter)

Comment 1

5 years ago
Created attachment 750137 [details]
testcase (asserts fatally when loaded)
(Reporter)

Comment 2

5 years ago
Created attachment 750138 [details]
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
tracking-firefox23: --- → ?
status-b2g18: --- → unaffected
status-firefox22: --- → unaffected
status-firefox23: --- → affected
status-firefox24: --- → affected
status-firefox-esr17: --- → unaffected
tracking-firefox24: --- → ?
Keywords: regression
Assignee: nobody → continuation
(Assignee)

Comment 6

5 years ago
Created attachment 750571 [details] [diff] [review]
patch

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+
(Assignee)

Comment 8

5 years ago
Created attachment 750598 [details] [diff] [review]
patch
Attachment #750571 - Attachment is obsolete: true
Attachment #750598 - Flags: review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
This can't land without security approval.
Keywords: checkin-needed
tracking-firefox23: ? → +
tracking-firefox24: ? → +
(Assignee)

Comment 10

5 years ago
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+
(Assignee)

Comment 15

5 years ago
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?
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/f74414ca211d
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f74414ca211d
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-firefox24: affected → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Attachment #750598 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/f4ee591fb02f
status-firefox23: affected → fixed
Whiteboard: [adv-main23-]
Group: core-security
You need to log in before you can comment on or make changes to this bug.