If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Invalid cast in nsSVGSVGElement::GetCurrentViewElement

RESOLVED FIXED in Firefox 18

Status

()

Core
SVG
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: Nils, Assigned: Robert Longson)

Tracking

({sec-critical})

Trunk
mozilla20
x86_64
Linux
sec-critical
Points:
---
Bug Flags:
sec-bounty +
in-testsuite ?

Firefox Tracking Flags

(firefox17 unaffected, firefox18+ fixed, firefox19+ fixed, firefox20 fixed, firefox-esr10 unaffected, firefox-esr17 unaffected)

Details

(Whiteboard: [adv-main18-])

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 687566 [details]
testcase.svg (crashes firefox)

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.
(Reporter)

Comment 1

5 years ago
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

5 years ago
Created attachment 688283 [details] [diff] [review]
patch
Assignee: nobody → longsonr
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #688283 - Flags: review?(dholbert)
Keywords: sec-critical
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?
Attachment #688283 - Flags: review?(dholbert) → review+
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 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+
tracking-firefox18: --- → +
tracking-firefox19: --- → +
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/247f62a63000

Comment 7

5 years ago
https://hg.mozilla.org/mozilla-central/rev/247f62a63000
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
status-firefox20: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
(Assignee)

Comment 8

5 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

5 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+
https://hg.mozilla.org/releases/mozilla-aurora/rev/583a68cdaaa8
https://hg.mozilla.org/releases/mozilla-beta/rev/2aff22c4baca
status-firefox18: affected → fixed
status-firefox19: affected → fixed
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.