Last Comment Bug 814001 - (CVE-2013-0753) [FIX] XMLSerializer Use-After-Free Remote Code Execution Vulnerability (ZDI-CAN-1608)
: [FIX] XMLSerializer Use-After-Free Remote Code Execution Vulnerability (ZDI-C...
: csectype-uaf, regression, sec-critical
Product: Core
Classification: Components
Component: DOM (show other bugs)
: 13 Branch
: All Windows XP
: -- normal (vote)
: mozilla20
Assigned To: Olli Pettay [:smaug]
: Andrew Overholt [:overholt]
Depends on:
Blocks: 873203
  Show dependency treegraph
Reported: 2012-11-21 08:01 PST by Curtis Koenig [:curtisk-use]]
Modified: 2013-05-16 12:34 PDT (History)
9 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

PoC (858 bytes, application/java-archive)
2012-11-21 08:04 PST, Curtis Koenig [:curtisk-use]]
no flags Details
patch (951 bytes, patch)
2012-11-21 10:06 PST, Olli Pettay [:smaug]
no flags Details | Diff | Splinter Review
alternative (3.38 KB, patch)
2012-11-21 12:01 PST, Olli Pettay [:smaug]
no flags Details | Diff | Splinter Review
random testing (74.09 KB, text/html)
2012-11-21 12:05 PST, Olli Pettay [:smaug]
no flags Details
patch (1.83 KB, patch)
2012-11-21 12:08 PST, Olli Pettay [:smaug]
bzbarsky: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
akeybl: approval‑mozilla‑esr17+
abillings: sec‑approval+
Details | Diff | Splinter Review
esr10 (1.90 KB, patch)
2012-12-10 09:41 PST, Olli Pettay [:smaug]
lukasblakk+bugs: approval‑mozilla‑esr10+
Details | Diff | Splinter Review

Description Curtis Koenig [:curtisk-use]] 2012-11-21 08:01:48 PST
::gpgmail-start-pgp-part::ZDI-CAN-1608: Mozilla Firefox XMLSerializer Use-After-Free Remote Code Execution Vulnerability

-- CVSS -----------------------------------------
7.5, AV:N/AC:L/Au:N/C:P/I:P/A:P

-- ABSTRACT -------------------------------------
TippingPoint has identified a vulnerability affecting the following products:

  Mozilla Firefox

-- VULNERABILITY DETAILS ------------------------
Version(s) Tested: Mozilla Firefox 13.0.1
Platform(s) Tested: Windows XP SP3

Vulnerability details 

From the researcher:
From content/base/src/nsDOMSerializer.cpp:

nsDocumentEncoder::SerializeToStringRecursive(nsINode* aNode,
nsAString& aStr,
bool aDontSerializeRoot) {
if (!aDontSerializeRoot) {
rv = SerializeNodeStart(maybeFixedNode, 0, -1, aStr, aNode);

nsDocumentEncoder::SerializeNodeStart(nsINode* aNode,
PRInt32 aStartOffset,
PRInt32 aEndOffset,
nsAString& aStr,
nsINode* aOriginalNode) {
if (node->IsElement()) {
Element* originalElement =
aOriginalNode && aOriginalNode->IsElement() ?
aOriginalNode->AsElement() : nsnull;
originalElement, aStr);
return NS_OK;

From content/base/src/nsXMLContentSerializer.cpp:

nsXMLContentSerializer::AppendElementStart(Element* aElement,
Element* aOriginalElement,
nsAString& aStr) {
if (!CheckElementStart(content, forceFormat, aStr)) {
return NS_OK;

From content/base/src/nsXHTMLContentSerializer.cpp:

nsXHTMLContentSerializer::CheckElementStart(nsIContent * aContent,
bool & aForceFormat,
nsAString& aStr) {
aForceFormat = aContent->HasAttr(kNameSpaceID_None,

After reaching the use-after-free condition, we need to take over memory
previously occupied by a deleted node. Thus, we force allocation of some
strings that will make |child->GetNextSibling()| return memory address within
predictable range of a heap spray. If data stored on a heap spray fulfils basic
constraints (0x8 at offset of |mBoolFlags| for the sake of |node->IsElement()|
being true), we are free to specify "custom" pointer to virtual function table.
This way we can arbitrarily change program execution flow at |aContent->HasAttr()|.

The code below triggers the vulnerability: 

Begin Code Snip 
<!doctype html>
     Mozilla Firefox 13.0.1
     XMLSerializer use-after-free
     Windows XP SP3 x86 EIP control Jul 2012

<body onload="run();">

End Code Snip 
This will create an XMLSerializer and an associated write function that will remove a node and spray the heap such that EIP will end up being c1c2c3c4

-- CREDIT ---------------------------------------
This vulnerability was discovered by:


-- FURTHER DETAILS ------------------------------
If supporting files were contained with this report they are provided within a password protected ZIP file. The password is the ZDI candidate number in the form: ZDI-CAN-XXXX where XXXX is the ID number.

Please confirm receipt of this report. We expect all vendors to remediate ZDI vulnerabilities within 180 days of the reported date. If you are ready to release a patch at any point leading up the the deadline please coordinate with us so that we may release our advisory detailing the issue. If the 180 day deadline is reached and no patch has been made available we will release a limited public advisory with our own mitigations so that the public can protect themselves in the absence of a patch. Please keep us updated regarding the status of this issue and feel free to contact us at any time:

Zero Day Initiative

The PGP key used for all ZDI vendor communications is available from:

-- INFORMATION ABOUT THE ZDI ---------------------
Established by TippingPoint, The Zero Day Initiative (ZDI) represents a best-of-breed model for rewarding security researchers for responsibly disclosing discovered vulnerabilities.

The ZDI is unique in how the acquired vulnerability information is used. TippingPoint does not re-sell the vulnerability details or any exploit code. Instead, upon notifying the affected product vendor, TippingPoint provides its customers with zero day protection through its intrusion prevention technology. Explicit details regarding the specifics of the vulnerability are not exposed to any parties until an official vendor patch is publicly available. Furthermore, with the altruistic aim of helping to secure a broader user base, TippingPoint provides this vulnerability information confidentially to security vendors (including competitors) who have a vulnerability protection or mitigation product.

Please contact us for further information or refer to:

-- DISCLOSURE POLICY ----------------------------
Our vulnerability disclosure policy is available online at:
Comment 1 Curtis Koenig [:curtisk-use]] 2012-11-21 08:04:20 PST
Created attachment 684027 [details]
Comment 2 Olli Pettay [:smaug] 2012-11-21 08:12:40 PST
Argh, we expose serializeToStream to web :/
Comment 3 Olli Pettay [:smaug] 2012-11-21 10:06:07 PST
Created attachment 684080 [details] [diff] [review]

I need to test this, but should work and be super safe.
The idea is that if serializeToStream is called by unsafe js, we just
write to the stream after the whole dom tree has been serialized.
Comment 4 Olli Pettay [:smaug] 2012-11-21 12:01:39 PST
Created attachment 684129 [details] [diff] [review]

might use less memory in some situations
Comment 5 Olli Pettay [:smaug] 2012-11-21 12:05:32 PST
Created attachment 684130 [details]
random testing
Comment 6 Olli Pettay [:smaug] 2012-11-21 12:08:09 PST
Created attachment 684133 [details] [diff] [review]

Make sure XHR doesn't use more memory than needed.
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2012-11-21 22:14:16 PST
Comment on attachment 684133 [details] [diff] [review]

I don't understand how this fixes the bug (or quite what the bug _is_ for that matter).  Can you explain?
Comment 8 Olli Pettay [:smaug] 2012-11-22 13:08:45 PST
(In reply to Boris Zbarsky (:bz) from comment #7)
> I don't understand how this fixes the bug (or quite what the bug _is_ for
> that matter).  Can you explain?
PoC is modifying DOM during outputstream write and serializer certainly doesn't expect
DOM changes while it is still serializing.
So, the patch makes us serialize the whole DOM (sub)tree to string and then call write (if caller isn't chrome).

We should hide serializeToStream from content, but we can't really do that on branches. So, I tried 
to find safer way to fix the crash.
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2012-11-26 19:56:29 PST
Comment on attachment 684133 [details] [diff] [review]

Oh, it's a JS-implemented "stream".  <sigh>.

Footguns on full automatic in action here....

Comment 10 Olli Pettay [:smaug] 2012-11-27 10:13:24 PST
Comment on attachment 684133 [details] [diff] [review]

[Security approval request comment]
How easily can the security issue be deduced from the patch?
Quite easily. 

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Not those, but the patch is making the problem reasonable obvious.

Which older supported branches are affected by this flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Same patch should work everywhere. ESR10 may need some merging to get XHR
related part correct. (That is anyway for perf and memusage reasons)

How likely is this patch to cause regressions; how much testing does it need?
Web pages shouldn't use serializeToStream. Other browsers don't support it.
But addons may use. However since addons run with chrome privs, their
behavior should stay the same. So, not likely to cause regressions.
Comment 11 Al Billings [:abillings] 2012-11-27 15:52:07 PST
Comment on attachment 684133 [details] [diff] [review]

sec-approval+ but we need you to wait until December 15 or later to check this in.
Comment 12 Daniel Veditz [:dveditz] 2012-11-27 16:46:13 PST
Created attachment 685892 [details]
ZDI POC (crashes on open in affected builds)

re-adding the PoC in a more usable form.

FWIW I don't get a crash in Firefox 10 ESR -- is this a regression from something? serializeToStream is there, though, so I guess that's bad enough to be worth fixing.
Comment 13 Daniel Veditz [:dveditz] 2012-11-27 16:50:56 PST
As far as I can tell this patch applies as is to ESR-10 so we should probably take it rather than waste time proving we don't need it.
Comment 14 Daniel Veditz [:dveditz] 2012-11-27 16:53:24 PST
I apologize for the flag-change spam. The fields are disappearing on me (listed read-only) and then that's counted as me setting them to blank?
Comment 15 Alex Keybl [:akeybl] 2012-11-29 15:46:06 PST
Please prepare patches for Aurora/Beta, and the ESR10/17 branches as well.
Comment 16 Olli Pettay [:smaug] 2012-12-10 05:59:15 PST
Comment 17 Olli Pettay [:smaug] 2012-12-10 06:00:30 PST
Comment on attachment 684133 [details] [diff] [review]

[Approval Request Comment]
Bug caused by (feature/regressing bug #): NA
User impact if declined: Ss crashes
Testing completed (on m-c, etc.): Landed to m-c, but m-c has now different
                                  serializer bindings
Risk to taking this patch (and alternatives if risky): should be safe 
String or UUID changes made by this patch: NA
Comment 18 Olli Pettay [:smaug] 2012-12-10 06:03:47 PST
Once this is in release builds, we should back out this from m-c, since the patch shouldn't be
with Paris bindings (the function doesn't exist anymore for web pages)
Comment 19 Alex Keybl [:akeybl] 2012-12-10 06:45:44 PST
Comment on attachment 684133 [details] [diff] [review]

Thanks for preparing these. I believe we're still looking for an ESR10 patch as well.
Comment 21 Olli Pettay [:smaug] 2012-12-10 09:41:50 PST
Created attachment 690438 [details] [diff] [review]
Comment 23 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-01-03 10:48:00 PST
Is there anything QA can do to shake out potential regressions in the upcoming releases (18, 10.0.12esr, 17.0.2esr)?
Comment 24 Olli Pettay [:smaug] 2013-01-03 11:05:46 PST
Check which addons use XMLSerializer and test them?
Comment 25 Olli Pettay [:smaug] 2013-01-03 11:06:11 PST
and/or DocumentEncoder
Comment 26 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-01-03 11:39:59 PST
(In reply to Olli Pettay [:smaug] from comment #24)
> Check which addons use XMLSerializer or DocumentEncoder and test them?

Jorge, can you advise us on which add-ons we might test?
Comment 28 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-01-04 16:14:47 PST
Thanks Jorge.

Olli, can you please advise us how we might use these addons to test this bug? I assume there's something more technical involved than simply installing the add-on and using it as intended.
Comment 29 Masatoshi Kimura [:emk] 2013-04-30 21:11:18 PDT
(In reply to Olli Pettay [:smaug] from comment #18)
> Once this is in release builds, we should back out this from m-c, since the
> patch shouldn't be
> with Paris bindings (the function doesn't exist anymore for web pages)

Can we back out now?

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