Closed Bug 814001 (CVE-2013-0753) Opened 12 years ago Closed 12 years ago

[FIX] XMLSerializer Use-After-Free Remote Code Execution Vulnerability (ZDI-CAN-1608)

Categories

(Core :: DOM: Core & HTML, defect)

13 Branch
All
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla20
Tracking Status
firefox17 --- wontfix
firefox18 + fixed
firefox19 + fixed
firefox20 + fixed
firefox-esr10 18+ fixed
firefox-esr17 18+ fixed

People

(Reporter: curtisk, Assigned: smaug)

References

Details

(Keywords: csectype-uaf, regression, sec-critical, Whiteboard: [adv-main18+][adv-esr17+][adv-esr10+][qa?])

Attachments

(5 files, 1 obsolete file)

::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); NS_ENSURE_SUCCESS(rv, rv); } ... } nsDocumentEncoder::SerializeNodeStart(nsINode* aNode, PRInt32 aStartOffset, PRInt32 aEndOffset, nsAString& aStr, nsINode* aOriginalNode) { ... if (node->IsElement()) { ... Element* originalElement = aOriginalNode && aOriginalNode->IsElement() ? aOriginalNode->AsElement() : nsnull; mSerializer->AppendElementStart(node->AsElement(), 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, nsGkAtoms::mozdirty); ... } 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 regenrecht@o2.pl Jul 2012 --> <html> <body onload="run();"> </body> </html> --------------- 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: regenrecht -- 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 zdi-disclosures@tippingpoint.com The PGP key used for all ZDI vendor communications is available from: http://www.zerodayinitiative.com/documents/zdi-pgp-key.asc -- 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: http://www.zerodayinitiative.com -- DISCLOSURE POLICY ---------------------------- Our vulnerability disclosure policy is available online at: http://www.zerodayinitiative.com/advisories/disclosure_policy/::gpgmail-end-pgp-part::
Attached file PoC (obsolete) —
Argh, we expose serializeToStream to web :/
Attachment #684027 - Attachment is private: true
Attachment #684027 - Attachment is private: false
Assignee: nobody → bugs
Alias: ZDI-CAN-1608
Summary: XMLSerializer Use-After-Free Remote Code Execution Vulnerability → XMLSerializer Use-After-Free Remote Code Execution Vulnerability (ZDI-CAN-1608)
Attached patch patchSplinter 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.
Attached patch alternativeSplinter Review
might use less memory in some situations
Attached file random testing
Attached patch patchSplinter Review
Make sure XHR doesn't use more memory than needed.
Attachment #684133 - Flags: review?(bzbarsky)
Comment on attachment 684133 [details] [diff] [review] patch I don't understand how this fixes the bug (or quite what the bug _is_ for that matter). Can you explain?
(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 on attachment 684133 [details] [diff] [review] patch Oh, it's a JS-implemented "stream". <sigh>. Footguns on full automatic in action here.... r=me
Attachment #684133 - Flags: review?(bzbarsky) → review+
Comment on attachment 684133 [details] [diff] [review] patch [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? All 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.
Attachment #684133 - Flags: sec-approval?
Comment on attachment 684133 [details] [diff] [review] patch sec-approval+ but we need you to wait until December 15 or later to check this in.
Attachment #684133 - Flags: sec-approval? → sec-approval+
Whiteboard: wait until after Dec 15 to land.
Attachment #684027 - Attachment mime type: application/zip → application/java-archive
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.
Attachment #684027 - Attachment is obsolete: true
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.
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?
Please prepare patches for Aurora/Beta, and the ESR10/17 branches as well.
Summary: XMLSerializer Use-After-Free Remote Code Execution Vulnerability (ZDI-CAN-1608) → [FIX] XMLSerializer Use-After-Free Remote Code Execution Vulnerability (ZDI-CAN-1608)
Whiteboard: wait until after Dec 15 to land. → wait until Dec 10 to land.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment on attachment 684133 [details] [diff] [review] patch [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
Attachment #684133 - Flags: approval-mozilla-esr17?
Attachment #684133 - Flags: approval-mozilla-beta?
Attachment #684133 - Flags: approval-mozilla-aurora?
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 on attachment 684133 [details] [diff] [review] patch Thanks for preparing these. I believe we're still looking for an ESR10 patch as well.
Attachment #684133 - Flags: approval-mozilla-esr17?
Attachment #684133 - Flags: approval-mozilla-esr17+
Attachment #684133 - Flags: approval-mozilla-beta?
Attachment #684133 - Flags: approval-mozilla-beta+
Attachment #684133 - Flags: approval-mozilla-aurora?
Attachment #684133 - Flags: approval-mozilla-aurora+
Attached patch esr10Splinter Review
Attachment #690438 - Flags: approval-mozilla-esr10?
Attachment #690438 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
Whiteboard: [adv-main18+][adv-esr17+][adv-esr10+]
Is there anything QA can do to shake out potential regressions in the upcoming releases (18, 10.0.12esr, 17.0.2esr)?
Whiteboard: [adv-main18+][adv-esr17+][adv-esr10+] → [adv-main18+][adv-esr17+][adv-esr10+][qa?]
Check which addons use XMLSerializer and test them?
and/or DocumentEncoder
(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?
Alias: CVE-2013-0753
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.
Group: core-security
(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?
Blocks: 873203
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: