Closed Bug 817439 Opened 11 years ago Closed 11 years ago

Invalid cast in nsSVGSVGElement::GetCurrentViewElement


(Core :: SVG, defect)

Not set



Tracking Status
firefox17 --- unaffected
firefox18 + fixed
firefox19 + fixed
firefox20 --- fixed
firefox-esr10 --- unaffected
firefox-esr17 --- unaffected


(Reporter: nils, Assigned: longsonr)



(Keywords: sec-critical, Whiteboard: [adv-main18-])


(2 files)

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

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:
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
Attached patch patchSplinter Review
Assignee: nobody → longsonr
Ever confirmed: true
Attachment #688283 - Flags: review?(dholbert)
This is code added in bug 783995, so this affects Firefox 18 & newer.  (Verified locally that Firefox 18 is affected & Firefox 17 is not)
Attachment #688283 - Flags: review?(dholbert) → review+
Comment on attachment 688283 [details] [diff] [review]

[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?


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 on attachment 688283 [details] [diff] [review]

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+
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Comment on attachment 688283 [details] [diff] [review]

[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 on attachment 688283 [details] [diff] [review]

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+
Whiteboard: [adv-main18-]
Flags: sec-bounty?
Flags: sec-bounty? → sec-bounty+
Group: core-security
You need to log in before you can comment on or make changes to this bug.