Beginning on October 25th, 2016, Persona will no longer be an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 572985 - (CVE-2010-1214) Plugin Parameter EnsureCachedAttrParamArrays Remote Code Execution Vulnerability (ZDI-CAN-821)
: Plugin Parameter EnsureCachedAttrParamArrays Remote Code Execution Vulnerabil...
: fixed1.9.0.20
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: unspecified
: All All
: -- critical (vote)
: ---
Assigned To: Josh Aas
: Benjamin Smedberg [:bsmedberg]
Depends on: CVE-2010-2755 580874
  Show dependency treegraph
Reported: 2010-06-18 00:27 PDT by Reed Loden [:reed] (use needinfo?)
Modified: 2010-08-10 11:59 PDT (History)
14 users (show)
dveditz: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

trunk fix v1.0 (14.09 KB, patch)
2010-06-21 22:00 PDT, Josh Aas
no flags Details | Diff | Splinter Review
trunk fix v1.1 (14.23 KB, patch)
2010-06-21 23:02 PDT, Josh Aas
no flags Details | Diff | Splinter Review
1.9.2 fix v1.0 (7.81 KB, patch)
2010-06-21 23:13 PDT, Josh Aas
no flags Details | Diff | Splinter Review
trunk fix v1.2 (14.42 KB, patch)
2010-06-22 09:53 PDT, Josh Aas
jst: review+
Details | Diff | Splinter Review
1.9.2 fix v1.1 (7.80 KB, patch)
2010-06-22 09:55 PDT, Josh Aas
jst: review+
dveditz: approval1.9.2.7+
Details | Diff | Splinter Review
1.9.1 fix v1.0 (5.28 KB, patch)
2010-06-22 12:59 PDT, Josh Aas
jst: review+
dveditz: approval1.9.1.11+
Details | Diff | Splinter Review

Description Reed Loden [:reed] (use needinfo?) 2010-06-18 00:27:14 PDT
Created attachment 452201 [details]

ZDI-CAN-821: Mozilla Firefox Plugin Parameter EnsureCachedAttrParamArrays Remote Code Execution Vulnerability

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

TippingPoint has identified a vulnerability affecting the following 

    Mozilla Firefox 3.6.x

-- 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 browser's method for parsing child
elements out of a particular tag. The application will use a 32-bit
index to enumerate them, but will store it in a 16-bit signed integer
and then use it to allocate space for a cache.  When populating the
cache a buffer overflow will occur. This can lead to code execution
under the context of the application.

The issue occurs within the application's support for parameters within
plugins. The application will create a cache for each parameter value
and name. This is done by enumerating the total number of parameters and
storing it. When calculating the size needed for the cache, the
application will store the total number of results into a signed
integer. This will cause the arithmetic required to undercalculate the
size for the allocation. Later when this cache is populated the buffer
overflow will occur.

After parsing the elements required for a plugin, the application will
enter the following code which will count the number of elements for the
cached Attribute/Param array.

nsresult nsPluginInstanceOwner::EnsureCachedAttrParamArrays()
  if (mCachedAttrParamValues)
    return NS_OK;

  NS_PRECONDITION(((mNumCachedAttrs + mNumCachedParams) == 0) &&
                  "re-cache of attrs/params not implemented! use the DOM
                  "node directy instead");

  // first, we need to find out how much we need to allocate for our
  // arrays count up attributes
  mNumCachedAttrs = 0;

  PRUint32 cattrs = mContent->GetAttrCount();

  if (cattrs < 0x0000FFFF) {
    // unsigned 32 bits to unsigned 16 bits conversion
    mNumCachedAttrs = static_cast<PRUint16>(cattrs);
  } else {
    mNumCachedAttrs = 0xFFFE;  // minus one in case we add an extra
"src" entry below

  // now, we need to find all the PARAM tags that are children of us
  // however, be carefull NOT to include any PARAMs that don't have us
  // as a direct parent. For nested object (or applet) tags, be sure
  // to only round up the param tags that coorespond with THIS
  // instance. And also, weed out any bogus tags that may get in the
  // way, see bug 39609. Then, with any param tag that meet our
  // qualification, temporarly cache them in an nsCOMArray until
  // we can figure out what size to make our fixed char* array.

  mNumCachedParams = 0;

After counting the number of elements, this will be assigned to a 16-bit
integer and then used in an allocation.

  if (allParams) {
    PRUint32 numAllParams; 
    // loop through every so called dependent PARAM tag to check if it
    // "belongs" to us

    for (PRUint32 i = 0; i < numAllParams; i++) {
      nsCOMPtr<nsIDOMNode> pnode;
      allParams->Item(i, getter_AddRefs(pnode));

      nsCOMPtr<nsIDOMElement> domelement = do_QueryInterface(pnode);
      if (domelement) {
            nsCOMPtr<nsIDOMNode> mydomNode =
            if (parent == mydomNode) {

  // We're done with DOM method calls now; make sure we still have a

  PRUint32 cparams = ourParams.Count(); // unsigned 32 bits to unsigned
16 bits conversion
  if (cparams < 0x0000FFFF)
    mNumCachedParams = static_cast<PRUint16>(cparams);
    mNumCachedParams = 0xFFFF;


  PRInt16 numRealAttrs = mNumCachedAttrs;           // XXX: implied type
conversion from unsigned to signed
  nsAutoString data;
  if (mContent->Tag() == nsGkAtoms::object
    && !mContent->HasAttr(kNameSpaceID_None, nsGkAtoms::src)
    && mContent->GetAttr(kNameSpaceID_None, nsGkAtoms::data, data)) {

  // now lets make the arrays
  mCachedAttrParamNames  = (char **)PR_Calloc(sizeof(char *) *
(mNumCachedAttrs + 1 + mNumCachedParams), 1);            // XXX: signed
integer being used in an allocation
  mCachedAttrParamValues = (char **)PR_Calloc(sizeof(char *) *
(mNumCachedAttrs + 1 + mNumCachedParams), 1);            // XXX: signed
integer being used in an allocation

The following code will then populate the allocated memory with the
cache. This will overflow the underallocated buffer that was provided.

  // Some plugins (eg Flash, see bug 234675.) are actually sensitive to
  // attribute order.  So we want to make sure we give the plugin the
  // attributes in the order they came in in the source, to be
compatible with
  // other browsers.  Now in HTML, the storage order is the reverse of
  // source order, while in XML and XHTML it's the same as the source
  // (see the AddAttributes functions in the HTML and XML content
  PRInt16 start, end, increment;
  if (mContent->IsNodeOfType(nsINode::eHTML) &&
      mContent->NodeInfo()->NamespaceEquals(kNameSpaceID_None)) {
    // HTML.  Walk attributes in reverse order.
    start = numRealAttrs - 1;
    end = -1;
    increment = -1;
  } else {
    // XHTML or XML.  Walk attributes in forward order.
    start = 0;
    end = numRealAttrs;
    increment = 1;
  for (PRInt16 index = start; index != end; index += increment) {
    const nsAttrName* attrName = mContent->GetAttrNameAt(index);
    nsIAtom* atom = attrName->LocalName();
    nsAutoString value;
    mContent->GetAttr(attrName->NamespaceID(), atom, value);
    nsAutoString name;

    FixUpURLS(name, value);

    mCachedAttrParamNames [c] = ToNewUTF8String(name);
    mCachedAttrParamValues[c] = ToNewUTF8String(value);

Version(s)  tested: Mozilla Firefox 3.6.3
Platform(s) tested: Windows XP SP3

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

This vulnerability was discovered by:
    * J23 (
Comment 1 Reed Loden [:reed] (use needinfo?) 2010-06-18 00:47:16 PDT

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a6pre) Gecko/20100617 Minefield/3.7a6pre
Comment 2 Daniel Veditz [:dveditz] 2010-06-21 10:53:23 PDT
roc: can you find an owner for this one?
Comment 3 Robert O'Callahan (:roc) (email my personal email if necessary) 2010-06-21 18:01:50 PDT
I think Josh should own this
Comment 4 Josh Aas 2010-06-21 22:00:56 PDT
Created attachment 452967 [details] [diff] [review]
trunk fix v1.0
Comment 5 Josh Aas 2010-06-21 23:02:33 PDT
Created attachment 452979 [details] [diff] [review]
trunk fix v1.1
Comment 6 Josh Aas 2010-06-21 23:13:23 PDT
Created attachment 452982 [details] [diff] [review]
1.9.2 fix v1.0

This is a minimal backport of the fix to the 1.9.2 branch. However, a browser fix might not be enough. This patch fixes the crash in the browser's code but then the JEP goes down - looks like it also has a bug. JEP is used on the 1.9.2 branch but not trunk. Will ask Steven Michaud to look into it, we may have to include an updated JEP.
Comment 7 Josh Aas 2010-06-22 08:52:07 PDT
Comment on attachment 452979 [details] [diff] [review]
trunk fix v1.1

Patch has a problem, new one coming up.
Comment 8 Steven Michaud [:smichaud] (Retired) 2010-06-22 08:55:13 PDT
Yup.  You forgot to reset nextAttrParamIndex to 0 before

+  for (PRUint16 i = 0; i < mNumCachedParams; i++) {
+    nsIDOMElement* param = ourParams.ObjectAt(i);
+    if (!param) {
+      continue;

I'll wait for your new patch.
Comment 9 Steven Michaud [:smichaud] (Retired) 2010-06-22 08:57:10 PDT
(I assume the problem I found is also the one you found.)
Comment 10 Steven Michaud [:smichaud] (Retired) 2010-06-22 09:00:35 PDT
Oops, scratch comment #8 -- I was wrong.
Comment 11 Josh Aas 2010-06-22 09:53:50 PDT
Created attachment 453092 [details] [diff] [review]
trunk fix v1.2
Comment 12 Josh Aas 2010-06-22 09:55:38 PDT
Created attachment 453093 [details] [diff] [review]
1.9.2 fix v1.1

This updated patch for 1.9.2 fixes the JEP crash as well.
Comment 13 Steven Michaud [:smichaud] (Retired) 2010-06-22 10:04:49 PDT
> This updated patch for 1.9.2 fixes the JEP crash as well.

Thanks.  Glad to hear it!
Comment 14 Josh Aas 2010-06-22 12:59:49 PDT
Created attachment 453150 [details] [diff] [review]
1.9.1 fix v1.0
Comment 15 Johnny Stenback (:jst, 2010-06-22 15:13:43 PDT
Is 1.9.1 actually vulnerable here?
Comment 16 Josh Aas 2010-06-23 07:10:10 PDT
1.9.1 is vulnerable, at least to a crash. The patch I posted fixes the problem.
Comment 17 Josh Aas 2010-06-23 12:11:01 PDT
pushed to mozilla-central
Comment 18 Daniel Veditz [:dveditz] 2010-06-23 15:37:23 PDT
FIXED on mozilla-central. We use the status1.9.1/status1.9.2 fields for branch resolution.
Comment 19 Daniel Veditz [:dveditz] 2010-06-23 15:38:53 PDT
"in-testsuite?" as a reminder to create/add the crash test once we've shipped this on the branches.
Comment 20 Daniel Veditz [:dveditz] 2010-06-23 16:16:33 PDT
Comment on attachment 453093 [details] [diff] [review]
1.9.2 fix v1.1

Approved for, a=dveditz for release-drivers
Comment 21 Daniel Veditz [:dveditz] 2010-06-23 16:16:49 PDT
Comment on attachment 453150 [details] [diff] [review]
1.9.1 fix v1.0

Approved for, a=dveditz for release-drivers
Comment 22 Josh Aas 2010-06-23 20:05:19 PDT
pushed to mozilla-1.9.2
Comment 23 Josh Aas 2010-06-24 08:44:32 PDT
pushed to mozilla-1.9.1
Comment 24 Al Billings [:abillings] 2010-07-01 17:42:56 PDT
On 1.9.1 and 1.9.2, I'm not seeing new behavior post-fix. I'm also not seeing a crash pre-fix.

On or with the PoC, the browser doesn't crash but goes up to 98-100% CPU usage and hangs there for a few minutes. Eventually, the CPU usage goes back to normal. With and build 1, I'm seeming the same behavior. This is on Windows XP. 

What am I missing in order to see the crash?
Comment 25 Josh Aas 2010-07-14 19:09:32 PDT
Al - can you test on Mac OS X? I was able to reproduce on 1.9.1, 1.9.2, and 1.9.3 on Mac OS X.
Comment 26 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-07-18 21:58:05 PDT
Comment on attachment 453150 [details] [diff] [review]
1.9.1 fix v1.0

Requesting on this patch so that we can take it in upcoming Camino 2.0.x security and stability updates.  If approved, I'll handle the checkins, unless the patch author requests otherwise.
Comment 27 Benjamin Smedberg [:bsmedberg] 2010-07-22 12:05:11 PDT
This caused regression bug 580874, I wouldn't take it on without the followup fix I'm going to attach there shortly.
Comment 28 Josh Aas 2010-07-22 12:43:43 PDT
I don't think this caused bug 580874, bug 575836 is the cause and that is a different than this (though in the same area of code).
Comment 29 Daniel Veditz [:dveditz] 2010-07-22 19:30:42 PDT
Comment on attachment 453150 [details] [diff] [review]
1.9.1 fix v1.0

Approved for, a=dveditz
Comment 30 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-07-25 22:11:48 PDT
Checking in layout/generic/nsObjectFrame.cpp;
/cvsroot/mozilla/layout/generic/nsObjectFrame.cpp,v  <--  nsObjectFrame.cpp
new revision: 1.660; previous revision: 1.659

Note You need to log in before you can comment on or make changes to this bug.