648090 - (CVE-2011-0083) Mozilla Firefox SVGPathSegList.replaceItem Remote Code Execution Vulnerability (ZDI-CAN-1142)

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Brandon Sterne (:bsterne)]

Attachment: PoC

ZDI-CAN-1142: Mozilla Firefox SVGPathSegList.replaceItem Remote Code Execution Vulnerability

-- CVSS ----------------------------------------------------------------
9, (AV:N/AC:L/Au:N/C:P/I:P/A:C)

-- ABSTRACT ------------------------------------------------------------

TippingPoint has identified a vulnerability affecting the following 
products:

    Mozilla Firefox

-- VULNERABILITY DETAILS -----------------------------------------------

This vulnerability allows remote attackers to execute arbitrary code on vulnerable installations of Mozilla Firefox. User interaction is
required to exploit this vulnerability in that the target must visit a malicious page or open a malicious file.

The specific flaw exists within the code responsible for parsing SVG path segment objects. The function nsSVGPathSegList::ReplaceItem() does
not account for deletion of the segment object list within a user defined DOMAttrModified EventListener. Code within
nsSVGPathSegList::ReplaceItem() references the segment list without verifying that it was not deleted in the aforementioned callback. This
can be abused to create a dangling reference which can be leveraged to execute arbitrary code within the context of the browser.

Version(s)  tested: 3.6.13
Platform(s) tested: Win XP SP3

From content/svg/content/src/nsSVGPathSegList.cpp:

NS_IMETHODIMP nsSVGPathSegList::ReplaceItem(nsIDOMSVGPathSeg *newItem,
                                            PRUint32 index,
                                            nsIDOMSVGPathSeg **_retval)
{
  ...

  InsertElementAt(newItemSeg, index);
 
RemoveFromCurrentList(static_cast<nsSVGPathSeg*>(mSegments.ObjectAt(index+1)));
  NS_ADDREF(*_retval = newItem);

  return NS_OK;
}

During execution of |InsertElementAt| "DOMAttrModified" event is dispatched. User's provided event handler will be called. During that callback it is possible to delete whole content of |mSegments|, e.g. by calling method clear() on pathSegList object. As |mSegments| is defined as |nsCOMArray| and method |ObjectAt| does not do out-of-bound check of index argument, previously |ObjectAt| freed memory will be referenced.


-- CREDIT --------------------------------------------------------------

This vulnerability was discovered by:
    * regenrecht

Comment 1

[Daniel Veditz [:dveditz]]

provisionally "critical" based on reputation, but looks like the PoC got stripped in mail.

Comment 2

[Brandon Sterne (:bsterne)]

I've sent mail to the reporter requesting a re-send of the PoC.

Comment 3

[Daniel Veditz [:dveditz]]

Attachment: PoC (try 2)

Comment 4

[Jonathan Watt [:jwatt]]

Classes that have this bug:

  nsSVGNumberList
  nsSVGPathSegList

Classes that do NOT have this bug:

  nsSVGLengthList - ReplaceItem() not implemented
  nsSVGPointList - ReplaceItem() not implemented
  nsSVGTransformList - Different implementation

Comment 5

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 6

[Daniel Holbert [:dholbert]]

Comment on attachment 525534 [details] [diff] [review]
patch

> nsSVGNumberList::ReplaceItem(nsIDOMSVGNumber *newItem,
>                              PRUint32 index,
>                              nsIDOMSVGNumber **_retval)
> {
>-  nsresult rv = RemoveItem(index, _retval);
>-  if (NS_FAILED(rv))
>-    return rv;
>+  if (index >= mNumbers.Length()) {
>+    return NS_ERROR_DOM_INDEX_SIZE_ERR;
>+  }
> 
>-  return InsertElementAt(newItem, index);
>+  WillModify();
>+  NS_REMOVE_SVGVALUE_OBSERVER(mNumbers[index]);
>+  NS_RELEASE(mNumbers[index]);
>+  mNumbers[index] = newItem;
>+  NS_ADDREF(newItem);
>+  NS_ADD_SVGVALUE_OBSERVER(newItem);
>+  DidModify();

This ^ is a bit hard to parse.  Could you add a comment saying something like:
> // This is equivalent to RemoveItem() followed by InsertElementAt(), but
> // with the WillModify() / DidModify() notifications collapsed.

r=dholbert either way

Comment 7

[Jonathan Watt [:jwatt]]

What's the procedure for landing this on 1.9.1 and 1.9.2? I now have patches for both ready to push.

Comment 8

[Brandon Sterne (:bsterne)]

Comment on attachment 525534 [details] [diff] [review]
patch

Granting approval for 1.9.2.18 and 1.9.1.20 (assuming the same patch applies there) for release-drivers.

Comment 9

[Jonathan Watt [:jwatt]]

Does approval mean I should push directly to 1.9.1 and 1.9.2, or is there some sort of shadow repo that I should be pushing to?

Comment 10

[Jonathan Watt [:jwatt]]

(In reply to comment #8)
> [snip] (assuming the same patch applies there) [snip]

The patch needs to be tweaked slightly, but it's essentially the same.

Comment 11

[Jonathan Watt [:jwatt]]

Daniel, can I direct comment 9 at you, or ask you to get the appropriate person to comment? Apparently security bugs aren't watched as closely as I expected so asking the wind doesn't work.

Comment 12

[Daniel Veditz [:dveditz]]

Yes, please check approved patches directly into their respective branches. (shadow repo will be for trunk, as a holding pen until branch patches are ready)

Comment 13

[Daniel Veditz [:dveditz]]

jst marked this tracking-firefox5+ but the version field says "1.9.2". Does this bug affect mozilla-central/aurora?

Comment 14

[Johnathan Nightingale [:johnath]]

jwatt - jst's out for a while, can you tell us whether this impacts aurora as well? If so, can you request approval-aurora and land it there when you land it elsewhere?

Comment 15

[Daniel Holbert [:dholbert]]

I'll defer to jwatt for the definitive answer, but AFAIK this doesn't affect Aurora / Firefox 5.  (nor does it affect Firefox 4)

The classes in question (comment 4) were completely rewritten after gecko 1.9.2

Comment 16

[Jonathan Watt [:jwatt]]

(In reply to comment #3)
> Created attachment 524301 [details]
> PoC (try 2)

This .zip is password protected, so I can't access it.

Comment 17

[Jonathan Watt [:jwatt]]

(In reply to comment #13)
> jst marked this tracking-firefox5+ but the version field says "1.9.2". Does
> this bug affect mozilla-central/aurora?

No. It only affects 1.9.2 and earlier.

Comment 18

[Jonathan Watt [:jwatt]]

Pushed http://hg.mozilla.org/releases/mozilla-1.9.2/rev/14dbc2f02f79
Pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e97fac945e09

Comment 19

[Al Billings [:abillings - ex-MoCo]]

Loading the PoC in 1.9.2.17, I get this crash: https://crash-stats.mozilla.com/report/index/43e40b3b-5762-4707-8995-165692110607

In 1.9.2.18pre (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.18pre) Gecko/20110607 Namoroka/3.6.18pre (.NET CLR 3.5.30729)), I get no crash and the PoC renders.

This looks to be fixed in 1.9.2.