Closed
Bug 572985
(CVE-2010-1214)
Opened 13 years ago
Closed 13 years ago
Plugin Parameter EnsureCachedAttrParamArrays Remote Code Execution Vulnerability (ZDI-CAN-821)
Categories
(Core Graveyard :: Plug-ins, defect)
Core Graveyard
Plug-ins
Tracking
(blocking2.0 beta2+, blocking1.9.2 .7+, status1.9.2 .7-fixed, blocking1.9.1 .11+, status1.9.1 .11-fixed)
People
(Reporter: reed, Assigned: jaas)
References
Details
(Keywords: fixed1.9.0.20, Whiteboard: [sg:critical?])
Attachments
(3 files, 3 obsolete files)
14.42 KB,
patch
|
jst
:
review+
|
Details | Diff | Splinter Review |
7.80 KB,
patch
|
jst
:
review+
dveditz
:
approval1.9.2.7+
|
Details | Diff | Splinter Review |
5.28 KB,
patch
|
jst
:
review+
dveditz
:
approval1.9.1.11+
dveditz
:
approval1.9.0.next+
|
Details | Diff | Splinter Review |
ZDI-CAN-821: Mozilla Firefox Plugin Parameter EnsureCachedAttrParamArrays Remote Code Execution Vulnerability -- ABSTRACT ------------------------------------------------------------ TippingPoint has identified a vulnerability affecting the following products: 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. layout/generic/nsObjectFrame.cpp:2873 nsresult nsPluginInstanceOwner::EnsureCachedAttrParamArrays() { if (mCachedAttrParamValues) return NS_OK; NS_PRECONDITION(((mNumCachedAttrs + mNumCachedParams) == 0) && !mCachedAttrParamNames, "re-cache of attrs/params not implemented! use the DOM " "node directy instead"); NS_ENSURE_TRUE(mOwner, NS_ERROR_NULL_POINTER); // 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. layout/generic/nsObjectFrame.cpp:2938 if (allParams) { PRUint32 numAllParams; allParams->GetLength(&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 = do_QueryInterface(mydomElement); if (parent == mydomNode) { ourParams.AppendObject(domelement); } } } } } } // We're done with DOM method calls now; make sure we still have a frame. NS_ENSURE_TRUE(mOwner, NS_ERROR_OUT_OF_MEMORY); PRUint32 cparams = ourParams.Count(); // unsigned 32 bits to unsigned 16 bits conversion if (cparams < 0x0000FFFF) mNumCachedParams = static_cast<PRUint16>(cparams); else 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)) { mNumCachedAttrs++; } // now lets make the arrays mCachedAttrParamNames = (char **)PR_Calloc(sizeof(char *) * (mNumCachedAttrs + 1 + mNumCachedParams), 1); // XXX: signed integer being used in an allocation NS_ENSURE_TRUE(mCachedAttrParamNames, NS_ERROR_OUT_OF_MEMORY); mCachedAttrParamValues = (char **)PR_Calloc(sizeof(char *) * (mNumCachedAttrs + 1 + mNumCachedParams), 1); // XXX: signed integer being used in an allocation NS_ENSURE_TRUE(mCachedAttrParamValues, NS_ERROR_OUT_OF_MEMORY); The following code will then populate the allocated memory with the cache. This will overflow the underallocated buffer that was provided. layout/generic/nsObjectFrame.cpp:3038 // Some plugins (eg Flash, see bug 234675.) are actually sensitive to the // 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 the // source order, while in XML and XHTML it's the same as the source order // (see the AddAttributes functions in the HTML and XML content sinks). 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; atom->ToString(name); FixUpURLS(name, value); mCachedAttrParamNames [c] = ToNewUTF8String(name); mCachedAttrParamValues[c] = ToNewUTF8String(value); c++; } Version(s) tested: Mozilla Firefox 3.6.3 Platform(s) tested: Windows XP SP3 -- CREDIT -------------------------------------------------------------- This vulnerability was discovered by: * J23 (http://twitter.com/HansJ23)
Reporter | ||
Comment 1•13 years ago
|
||
bp-5d5557d0-e6b4-45c0-8237-0ff972100618 Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a6pre) Gecko/20100617 Minefield/3.7a6pre
Updated•13 years ago
|
blocking1.9.2: ? → needed
I think Josh should own this
Assignee: roc → joshmoz
Attachment #452967 -
Attachment description: fix v1.0 → trunk fix v1.0
Attachment #452967 -
Attachment is obsolete: true
Attachment #452979 -
Flags: review?(jst)
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.
Attachment #452982 -
Flags: review?(jst)
Comment on attachment 452979 [details] [diff] [review] trunk fix v1.1 Patch has a problem, new one coming up.
Attachment #452979 -
Attachment is obsolete: true
Attachment #452979 -
Flags: review?(jst)
Attachment #452982 -
Attachment is obsolete: true
Attachment #452982 -
Flags: review?(jst)
Comment 8•13 years ago
|
||
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•13 years ago
|
||
(I assume the problem I found is also the one you found.)
Comment 10•13 years ago
|
||
Oops, scratch comment #8 -- I was wrong.
Assignee | ||
Comment 11•13 years ago
|
||
Attachment #453092 -
Flags: review?(jst)
Assignee | ||
Comment 12•13 years ago
|
||
This updated patch for 1.9.2 fixes the JEP crash as well.
Attachment #453093 -
Flags: review?(jst)
Comment 13•13 years ago
|
||
> This updated patch for 1.9.2 fixes the JEP crash as well.
Thanks. Glad to hear it!
Assignee | ||
Comment 14•13 years ago
|
||
Attachment #453150 -
Flags: review?(jst)
Updated•13 years ago
|
Attachment #453092 -
Flags: review?(jst) → review+
Updated•13 years ago
|
Attachment #453093 -
Flags: review?(jst) → review+
Comment 15•13 years ago
|
||
Is 1.9.1 actually vulnerable here?
Assignee | ||
Comment 16•13 years ago
|
||
1.9.1 is vulnerable, at least to a crash. The patch I posted fixes the problem.
Updated•13 years ago
|
blocking1.9.1: ? → needed
Assignee | ||
Comment 17•13 years ago
|
||
pushed to mozilla-central http://hg.mozilla.org/mozilla-central/rev/fae939d0a47a
Status: NEW → ASSIGNED
Updated•13 years ago
|
Attachment #453150 -
Flags: review?(jst) → review+
Comment 18•13 years ago
|
||
FIXED on mozilla-central. We use the status1.9.1/status1.9.2 fields for branch resolution.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 19•13 years ago
|
||
"in-testsuite?" as a reminder to create/add the crash test once we've shipped this on the branches.
Flags: in-testsuite?
Updated•13 years ago
|
blocking1.9.1: needed → .11+
blocking1.9.2: needed → .6+
Updated•13 years ago
|
Attachment #453093 -
Flags: approval1.9.2.6+
Comment 20•13 years ago
|
||
Comment on attachment 453093 [details] [diff] [review] 1.9.2 fix v1.1 Approved for 1.9.2.6, a=dveditz for release-drivers
Comment 21•13 years ago
|
||
Comment on attachment 453150 [details] [diff] [review] 1.9.1 fix v1.0 Approved for 1.9.1.11, a=dveditz for release-drivers
Attachment #453150 -
Flags: approval1.9.1.11+
Assignee | ||
Comment 22•13 years ago
|
||
pushed to mozilla-1.9.2 http://hg.mozilla.org/releases/mozilla-1.9.2/rev/c6843f08a56f
Assignee | ||
Comment 23•13 years ago
|
||
pushed to mozilla-1.9.1 http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e93d30b2790d
Comment 24•13 years ago
|
||
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 1.9.1.10 or 1.9.2.6 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 1.9.1.11 and 1.9.2.7 build 1, I'm seeming the same behavior. This is on Windows XP. What am I missing in order to see the crash?
Updated•13 years ago
|
Alias: CVE-2010-1214
Assignee | ||
Comment 25•13 years ago
|
||
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 on attachment 453150 [details] [diff] [review] 1.9.1 fix v1.0 Requesting approval1.9.0.next 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.
Attachment #453150 -
Flags: approval1.9.0.next?
Comment 27•13 years ago
|
||
This caused regression bug 580874, I wouldn't take it on 1.9.0.next without the followup fix I'm going to attach there shortly.
Updated•13 years ago
|
Assignee | ||
Comment 28•13 years ago
|
||
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).
Updated•13 years ago
|
Depends on: CVE-2010-2755
Comment 29•13 years ago
|
||
Comment on attachment 453150 [details] [diff] [review] 1.9.1 fix v1.0 Approved for 1.9.0.20, a=dveditz
Attachment #453150 -
Flags: approval1.9.0.next? → approval1.9.0.next+
Checking in layout/generic/nsObjectFrame.cpp; /cvsroot/mozilla/layout/generic/nsObjectFrame.cpp,v <-- nsObjectFrame.cpp new revision: 1.660; previous revision: 1.659 done
Keywords: fixed1.9.0.20
Updated•13 years ago
|
Group: core-security
Updated•10 months ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•