Closed Bug 872812 Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: SVG, defect)

Product:
Core
Component:
SVG
Platform:
x86_64
macOS
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

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

People

(Reporter: jruderman, Assigned: baku)

References

Details

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

Attachments

(3 files, 1 obsolete file)

testcase (asserts fatally when loaded)
273 bytes, text/html
Details
stack (gdb)
12.41 KB, text/plain
Details
patch
2.69 KB, patch
baku
: review+
lsblakk
: approval-mozilla-aurora+
abillings
: sec-approval+
Details | Diff | Splinter Review
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 patch — — Splinter 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: 12 years ago
status-firefox24: affected → fixed
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.

Attachment

General

Creator:
Created:
Updated:
Size: