Closed
Bug 817439
Opened 11 years ago
Closed 11 years ago
Invalid cast in nsSVGSVGElement::GetCurrentViewElement
Categories
(Core :: SVG, defect)
Tracking
()
RESOLVED
FIXED
mozilla20
Tracking | Status | |
---|---|---|
firefox17 | --- | unaffected |
firefox18 | + | fixed |
firefox19 | + | fixed |
firefox20 | --- | fixed |
firefox-esr10 | --- | unaffected |
firefox-esr17 | --- | unaffected |
People
(Reporter: nils, Assigned: longsonr)
References
Details
(Keywords: sec-critical, Whiteboard: [adv-main18-])
Attachments
(2 files)
719 bytes,
image/svg+xml
|
Details | |
1.63 KB,
patch
|
dholbert
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:20.0) Gecko/20121130 Firefox/20.0 Build ID: 20121130030747 Steps to reproduce: Invalid cast in nsSVGSVGElement::GetCurrentViewElement Description: The code in nsSVGSVGElement::GetCurrentViewElement doesn't check whether the retrieved view element is a SVG element before casting it to nsSVGViewElement. By appending for example a HTML element with the tag name "view" to the SVG document with the same ID as the current view a crash can be triggered. The crash shows potentially exploitable behaviour. Testcase is attached (crashes the browser) Vulnerable Code: nsSVGViewElement* nsSVGSVGElement::GetCurrentViewElement() const { if (mCurrentViewID) { nsIDocument* doc = GetCurrentDoc(); if (doc) { Element *element = doc->GetElementById(*mCurrentViewID); if(element) { } if (element && element->Tag() == nsGkAtoms::view) { return static_cast<nsSVGViewElement*>(element); } } } return nullptr; } Tested and Affected Versions: Firefox 20.0a1 (trunk) reported by nils of vulndev ltd.
The testcase might need a few reloads to trigger the crash. Example crash on linux: Program received signal SIGSEGV, Segmentation fault. nsSVGOuterSVGFrame::GetIntrinsicRatio (this=0x7fffddf96ab8) at /home/albert/src/hg/src/layout/svg/nsSVGOuterSVGFrame.cpp:307 307 viewBoxHeight = 0.0f; (gdb) bt 10 #0 nsSVGOuterSVGFrame::GetIntrinsicRatio (this=0x7fffddf96ab8) at /home/albert/src/hg/src/layout/svg/nsSVGOuterSVGFrame.cpp:307 #1 0x00007ffff38a179b in nsSVGOuterSVGFrame::ComputeSize (this=0x7fffddf96ab8, aRenderingContext=0x7fffd5386480, aCBSize=..., aAvailableWidth=<optimised out>, aMargin=..., aBorder=..., aPadding=..., aFlags=0) at /home/albert/src/hg/src/layout/svg/nsSVGOuterSVGFrame.cpp:384 #2 0x00007ffff33d2408 in nsHTMLReflowState::InitConstraints (this=0x7fffffffab88, aPresContext=<optimised out>, aContainingBlockWidth=50400, aContainingBlockHeight=53280, aBorder=<optimised out>, aPadding=<optimised out>, aFrameType=0x7fffe3f11b80) at /home/albert/src/hg/src/layout/generic/nsHTMLReflowState.cpp:2064 #3 0x00007ffff33d2bd4 in nsHTMLReflowState::Init (this=0x7fffffffab88, aPresContext=0x7fffca49c000, aContainingBlockWidth=-1, aContainingBlockHeight=-1, aBorder=0x0, aPadding=0x0) at /home/albert/src/hg/src/layout/generic/nsHTMLReflowState.cpp:316 #4 0x00007ffff33cf14a in nsCanvasFrame::Reflow (this=0x7fffddefd7c0, aPresContext=0x7fffca49c000, aDesiredSize=..., aReflowState=..., aStatus=@0x7fffffffaecc: 0) at /home/albert/src/hg/src/layout/generic/nsCanvasFrame.cpp:454 #5 0x00007ffff33b30af in nsContainerFrame::ReflowChild (this=<optimised out>, aKidFrame=0x7fffddefd7c0, aPresContext=0x7fffca49c000, aDesiredSize=..., aReflowState=..., aX=<optimised out>, aY=0, aFlags=3, aStatus=@0x7fffffffaecc: 0, aTracker=0x0) at /home/albert/src/hg/src/layout/generic/nsContainerFrame.cpp:953 #6 0x00007ffff33c77f5 in nsHTMLScrollFrame::ReflowScrolledFrame (this=0x7fffddefd8d0, aState=0x7fffffffaff8, aAssumeHScroll=false, aAssumeVScroll=false, aMetrics=0x7fffffffaf20, aFirstPass=<optimised out>) at /home/albert/src/hg/src/layout/generic/nsGfxScrollFrame.cpp:435 #7 0x00007ffff33c9937 in nsHTMLScrollFrame::ReflowContents (this=0x7fffddefd8d0, aState=0x7fffffffaff8, aDesiredSize=...) at /home/albert/src/hg/src/layout/generic/nsGfxScrollFrame.cpp:533 #8 0x00007ffff33cdcb8 in nsHTMLScrollFrame::Reflow (this=0x7fffddefd8d0, aPresContext=0x7fffca49c000, aDesiredSize=..., aReflowState=..., aStatus=@0x7fffffffb59c: 0) at /home/albert/src/hg/src/layout/generic/nsGfxScrollFrame.cpp:774 #9 0x00007ffff33b30af in nsContainerFrame::ReflowChild (this=<optimised out>, aKidFrame=0x7fffddefd8d0, aPresContext=0x7fffca49c000, aDesiredSize=..., aReflowState=..., aX=<optimised out>, aY=0, aFlags=0, aStatus=@0x7fffffffb59c: 0, aTracker=0x0) at /home/albert/src/hg/src/layout/generic/nsContainerFrame.cpp:953 (More stack frames follow...) (gdb) x/10i $rip => 0x7ffff38a10d3 <nsSVGOuterSVGFrame::GetIntrinsicRatio()+189>: maxss 0xc(%rdx),%xmm0 0x7ffff38a10d8 <nsSVGOuterSVGFrame::GetIntrinsicRatio()+194>: maxss 0x8(%rdx),%xmm1 0x7ffff38a10dd <nsSVGOuterSVGFrame::GetIntrinsicRatio()+199>: movss %xmm1,0xc(%rsp) 0x7ffff38a10e3 <nsSVGOuterSVGFrame::GetIntrinsicRatio()+205>: callq 0x7ffff334a178 <NSToCoordRoundWithClamp(float)> 0x7ffff38a10e8 <nsSVGOuterSVGFrame::GetIntrinsicRatio()+210>: movss 0xc(%rsp),%xmm0 0x7ffff38a10ee <nsSVGOuterSVGFrame::GetIntrinsicRatio()+216>: mov %eax,%ebx 0x7ffff38a10f0 <nsSVGOuterSVGFrame::GetIntrinsicRatio()+218>: shl $0x20,%rbx 0x7ffff38a10f4 <nsSVGOuterSVGFrame::GetIntrinsicRatio()+222>: callq 0x7ffff334a178 <NSToCoordRoundWithClamp(float)> 0x7ffff38a10f9 <nsSVGOuterSVGFrame::GetIntrinsicRatio()+227>: mov %eax,%eax 0x7ffff38a10fb <nsSVGOuterSVGFrame::GetIntrinsicRatio()+229>: or %rbx,%rax (gdb) info reg rax 0x7fffc66d4f98 140736522440600 rbx 0x7fffc63e3560 140736519353696 rcx 0x7ffff70e491d 140737338296605 rdx 0x2e746e656d686361 3347421804949431137 rsi 0x25 37 rdi 0x7fffffffa1c0 140737488331200 rbp 0x7fffddf96ab8 0x7fffddf96ab8 rsp 0x7fffffffa990 0x7fffffffa990 r8 0x7ffff7fc7740 140737353905984 r9 0x7ffff7fc7740 140737353905984 r10 0x0 0 r11 0x0 0 r12 0xc4e0 50400 r13 0x0 0 r14 0x7fffc63e3560 140736519353696 r15 0xd02000000000 228835857530880 rip 0x7ffff38a10d3 0x7ffff38a10d3 <nsSVGOuterSVGFrame::GetIntrinsicRatio()+189> eflags 0x10202 [ IF RF ] cs 0x33 51 ss 0x2b 43 ds 0x0 0 es 0x0 0 fs 0x0 0 gs 0x0 0 (gdb)
Assignee | ||
Comment 2•11 years ago
|
||
Assignee: nobody → longsonr
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #688283 -
Flags: review?(dholbert)
Updated•11 years ago
|
Keywords: sec-critical
Comment 3•11 years ago
|
||
This is code added in bug 783995, so this affects Firefox 18 & newer. (Verified locally that Firefox 18 is affected & Firefox 17 is not)
Blocks: 783995
status-firefox-esr10:
--- → unaffected
status-firefox17:
--- → unaffected
status-firefox18:
--- → affected
status-firefox19:
--- → affected
status-firefox20:
--- → affected
status-firefox-esr17:
--- → unaffected
Flags: in-testsuite?
Updated•11 years ago
|
Attachment #688283 -
Flags: review?(dholbert) → review+
Comment 4•11 years ago
|
||
Comment on attachment 688283 [details] [diff] [review] patch [Security approval request comment] How easily can the security issue be deduced from the patch? Reasonably easily. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No. Which older supported branches are affected by this flaw? Firefox 18 & 19 (current Beta & Aurora) If not all supported branches, which bug introduced the flaw? bug 783995 Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Yes, backports should be trivial/identical. How likely is this patch to cause regressions; how much testing does it need? Unlikely. Won't need much testing.
Attachment #688283 -
Flags: sec-approval?
Comment 5•11 years ago
|
||
Comment on attachment 688283 [details] [diff] [review] patch Please get this in so it can be approved to go in the 18 beta and 19 aurora builds.
Attachment #688283 -
Flags: sec-approval? → sec-approval+
Updated•11 years ago
|
tracking-firefox18:
--- → +
tracking-firefox19:
--- → +
Assignee | ||
Comment 6•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/247f62a63000
Comment 7•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/247f62a63000
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Assignee | ||
Comment 8•11 years ago
|
||
Comment on attachment 688283 [details] [diff] [review] patch [Approval Request Comment] Bug caused by (feature/regressing bug #): 783995 User impact if declined: exploitable security hole Testing completed (on m-c, etc.): landed. Checked that testcase does not cause problems with patched build Risk to taking this patch (and alternatives if risky): pretty low. String or UUID changes made by this patch: none
Attachment #688283 -
Flags: approval-mozilla-beta?
Attachment #688283 -
Flags: approval-mozilla-aurora?
Comment 9•11 years ago
|
||
Comment on attachment 688283 [details] [diff] [review] patch Fix for an sg:crit regression as we're coming up to beta 4 - manageable risk. Approving for branches.
Attachment #688283 -
Flags: approval-mozilla-beta?
Attachment #688283 -
Flags: approval-mozilla-beta+
Attachment #688283 -
Flags: approval-mozilla-aurora?
Attachment #688283 -
Flags: approval-mozilla-aurora+
Comment 10•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/583a68cdaaa8 https://hg.mozilla.org/releases/mozilla-beta/rev/2aff22c4baca
Updated•11 years ago
|
Whiteboard: [adv-main18-]
Updated•11 years ago
|
Flags: sec-bounty?
Updated•11 years ago
|
Flags: sec-bounty? → sec-bounty+
Updated•11 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•