Closed
Bug 885539
Opened 11 years ago
Closed 11 years ago
Heap-use-after-free in nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyElements<mozilla::dom::HTMLImageElement*> >::Hdr()
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla25
Tracking | Status | |
---|---|---|
firefox23 | --- | unaffected |
firefox24 | + | fixed |
firefox25 | + | fixed |
firefox-esr17 | --- | unaffected |
b2g18 | --- | unaffected |
People
(Reporter: attekett, Assigned: baku)
References
Details
(4 keywords, Whiteboard: [asan][adv-main24-])
Attachments
(2 files)
3.02 KB,
text/plain
|
Details | |
1.22 KB,
patch
|
bzbarsky
:
review+
bajaj
:
approval-mozilla-beta+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
Tested on: OS: Ubuntu 12.04 Firefox: ASAN debug-build from https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-dbg-asan/1371733138/ ASAN-report: ###!!! ASSERTION: mForm should be null at this point!: '!mForm', file /builds/slave/m-cen-l64-dbg-asan-00000000000/build/content/html/content/src/nsGenericHTMLElement.cpp, line 2124 ###!!! ASSERTION: Form control should have had flag set correctly: '(mForm != nullptr) == HasFlag(ADDED_TO_FORM)', file /builds/slave/m-cen-l64-dbg-asan-00000000000/build/content/html/content/src/HTMLImageElement.cpp, line 664 ================================================================= ==17530== ERROR: AddressSanitizer: heap-use-after-free on address 0x7f38e07e45d0 at pc 0x7f39011b2c97 bp 0x7fffc993f410 sp 0x7fffc993f408 READ of size 8 at 0x7f38e07e45d0 thread T0 #0 0x7f39011b2c96 in nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyElements<mozilla::dom::HTMLImageElement*> >::Hdr() const /builds/slave/m-cen-l64-dbg-asan-00000000000/build/../../../../dist/include/nsTArray.h:498 #1 0x7f39011b2d38 in nsTArray_Impl<mozilla::dom::HTMLImageElement*, nsTArrayInfallibleAllocator>::Elements() const /builds/slave/m-cen-l64-dbg-asan-00000000000/build/../../../../dist/include/nsTArray.h:859 #2 0x7f39011b2cba in unsigned int nsTArray_Impl<mozilla::dom::HTMLImageElement*, nsTArrayInfallibleAllocator>::IndexOf<mozilla::dom::HTMLImageElement*, nsDefaultComparator<mozilla::dom::HTMLImageElement*, mozilla::dom::HTMLImageElement*> >(mozilla::dom::HTMLImageElement* const&, unsigned int, nsDefaultComparator<mozilla::dom::HTMLImageElement*, mozilla::dom::HTMLImageElement*> const&) const /builds/slave/m-cen-l64-dbg-asan-00000000000/build/../../../../dist/include/nsTArray.h:961 #3 0x7f39011aeb19 in unsigned int nsTArray_Impl<mozilla::dom::HTMLImageElement*, nsTArrayInfallibleAllocator>::IndexOf<mozilla::dom::HTMLImageElement*>(mozilla::dom::HTMLImageElement* const&, unsigned int) const /builds/slave/m-cen-l64-dbg-asan-00000000000/build/../../../../dist/include/nsTArray.h:977 #4 0x7f39011aea56 in mozilla::dom::HTMLFormElement::RemoveImageElement(mozilla::dom::HTMLImageElement*) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/content/html/content/src/HTMLFormElement.cpp:2789 #5 0x7f390104931f in mozilla::dom::HTMLImageElement::ClearForm(bool) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/content/html/content/src/HTMLImageElement.cpp:675 . . . freed by thread T0 here: #0 0x43b1e0 in free ??:0 #1 0x7f3900e2a0f7 in nsNodeUtils::LastRelease(nsINode*) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/content/base/src/nsNodeUtils.cpp:265 #2 0x7f3900c742c0 in mozilla::dom::FragmentOrElement::Release() /builds/slave/m-cen-l64-dbg-asan-00000000000/build/content/base/src/FragmentOrElement.cpp:1709 #3 0x7f39011a1ece in mozilla::dom::HTMLFormElement::Release() /builds/slave/m-cen-l64-dbg-asan-00000000000/build/content/html/content/src/HTMLFormElement.cpp:328 #4 0x7f38ffe32d67 in nsTArray_Impl<nsCOMPtr<nsIContent>, nsTArrayInfallibleAllocator>::DestructRange(unsigned int, unsigned int) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/../../dist/include/nsTArray.h:1535 #5 0x7f38ffe32c06 in nsTArray_Impl<nsCOMPtr<nsIContent>, nsTArrayInfallibleAllocator>::RemoveElementsAt(unsigned int, unsigned int) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/../../dist/include/nsTArray.h:1252 . . .
Updated•11 years ago
|
Severity: normal → critical
Component: General → DOM: Core & HTML
Keywords: testcase
Product: Firefox → Core
Whiteboard: [asan]
Assignee | ||
Comment 2•11 years ago
|
||
This bug is reprodicible with <img> and with <input>. It seems to be a bug in the parser. Un/BindToTree() has not been called yet. #1 0x00007ffff21a8670 in Break (aMsg= 0x7fffffffbd70 "###!!! ASSERTION: mForm should be null at this point!: '!mForm', file /home/baku/Sources/m/blob/src/content/html/content/src/nsGenericHTMLElement.cpp, line 2125") at /home/baku/Sources/m/blob/src/xpcom/base/nsDebugImpl.cpp:555 #2 0x00007ffff21a8623 in NS_DebugBreak (aSeverity=1, aStr=0x7ffff37d3cc8 "mForm should be null at this point!", aExpr=0x7ffff37d3cbc "!mForm", aFile= 0x7ffff37d37b8 "/home/baku/Sources/m/blob/src/content/html/content/src/nsGenericHTMLElement.cpp", aLine=2125) at /home/baku/Sources/m/blob/src/xpcom/base/nsDebugImpl.cpp:422 #3 0x00007ffff0b1a7c2 in nsGenericHTMLFormElement::~nsGenericHTMLFormElement (this=0x2108350, __in_chrg=<optimized out>) at /home/baku/Sources/m/blob/src/content/html/content/src/nsGenericHTMLElement.cpp:2125 #4 0x00007ffff0a3d65d in mozilla::dom::HTMLInputElement::~HTMLInputElement (this=0x2108350, __in_chrg=<optimized out>) at /home/baku/Sources/m/blob/src/content/html/content/src/HTMLInputElement.cpp:668 #5 0x00007ffff0a3d6b0 in mozilla::dom::HTMLInputElement::~HTMLInputElement (this=0x2108350, __in_chrg=<optimized out>) at /home/baku/Sources/m/blob/src/content/html/content/src/HTMLInputElement.cpp:675 #6 0x00007ffff087ea76 in nsNodeUtils::LastRelease (aNode=0x2108350) at /home/baku/Sources/m/blob/src/content/base/src/nsNodeUtils.cpp:265 #7 0x00007ffff07512ec in mozilla::dom::FragmentOrElement::Release (this=0x2108350) at /home/baku/Sources/m/blob/src/content/base/src/FragmentOrElement.cpp:1709 #8 0x00007ffff0a3db02 in mozilla::dom::HTMLInputElement::Release (this=0x2108350) at /home/baku/Sources/m/blob/src/content/html/content/src/HTMLInputElement.cpp:733 #9 0x00007fffefe50ebd in nsCOMPtr<nsIContent>::~nsCOMPtr (this=0x20f7110, __in_chrg=<optimized out>) at ../../../dist/include/nsCOMPtr.h:534 #10 0x00007fffefe58403 in nsTArrayElementTraits<nsCOMPtr<nsIContent> >::Destruct (e=0x20f7110) at ../../../dist/include/nsTArray.h:534 #11 0x00007fffefe5789d in nsTArray_Impl<nsCOMPtr<nsIContent>, nsTArrayInfallibleAllocator>::DestructRange (this=0x1fd8280, start=0, count=216) at ../../../dist/include/nsTArray.h:1535 #12 0x00007fffefe56068 in nsTArray_Impl<nsCOMPtr<nsIContent>, nsTArrayInfallibleAllocator>::RemoveElementsAt (this=0x1fd8280, start=0, count=216) at ../../../dist/include/nsTArray.h:1252 #13 0x00007fffefe5483d in nsTArray_Impl<nsCOMPtr<nsIContent>, nsTArrayInfallibleAllocator>::Clear (this=0x1fd8280) at ../../../dist/include/nsTArray.h:1263 #14 0x00007fffefe53f04 in nsTArray_Impl<nsCOMPtr<nsIContent>, nsTArrayInfallibleAllocator>::~nsTArray_Impl (this=0x1fd8280, __in_chrg=<optimized out>) at ../../../dist/include/nsTArray.h:748 #15 0x00007fffefe52044 in nsTArray<nsCOMPtr<nsIContent> >::~nsTArray (this=0x1fd8280, __in_chrg=<optimized out>) at ../../../dist/include/nsTArray.h:1608 #16 0x00007ffff10b1c4e in nsHtml5TreeOpExecutor::~nsHtml5TreeOpExecutor (this=0x1fd8180, __in_chrg=<optimized out>) at /home/baku/Sources/m/blob/src/parser/html/nsHtml5TreeOpExecutor.cpp:76
Updated•11 years ago
|
Flags: needinfo?(hsivonen)
Comment 3•11 years ago
|
||
Does the <input> version of this happen in older releases?
Updated•11 years ago
|
Keywords: csec-uaf,
sec-critical
Comment 4•11 years ago
|
||
> Un/BindToTree() has not been called yet.
Is refcounting sensitive to Un/BindToTree() having been called? That would be ... surprising.
OTOH, if this is a general refcounting bug in the tree op executor, why hasn't it failed spectacularly on pretty much everything earlier?
Updated•11 years ago
|
Updated•11 years ago
|
status-firefox25:
--- → affected
tracking-firefox25:
--- → ?
Updated•11 years ago
|
Flags: sec-bounty?
Assignee | ||
Comment 5•11 years ago
|
||
a###!!! ASSERTION: mForm should be null at this point!: '!mForm', file /home/baku/Sources/m/mozilla-b2g18/src/content/html/content/src/nsGenericHTMLElement.cpp, line 3146 WARNING: NS_ENSURE_TRUE(parentObject) failed: file /home/baku/Sources/m/mozilla-b2g18/src/accessible/src/atk/AccessibleWrap.cpp, line 1348 I see the same problem om mozilla-b2g18 where all these HTMLFormElement webIDL patches are not landed.
Comment 7•11 years ago
|
||
> Is refcounting sensitive to Un/BindToTree() having been called?
Refcounting no, but the maintenance of the weak ref to mForm might be.
Andrea, do we get the heap-use-after-free on b2g18 as well?
Comment 8•11 years ago
|
||
Andrea, are you planning to fix this on 24? If not, what's the plan there?
Flags: needinfo?(amarchesini)
Assignee | ||
Comment 9•11 years ago
|
||
(In reply to Boris Zbarsky (:bz) (reading mail, but on paternity leave) from comment #8) > Andrea, are you planning to fix this on 24? If not, what's the plan there? Yes I am. I'm working on this.
Flags: needinfo?(amarchesini)
Updated•11 years ago
|
Assignee: nobody → amarchesini
Assignee | ||
Comment 11•11 years ago
|
||
Bz, I know you are not available in this time, but I don't know who else can review this patch. Can you reassign it?
Attachment #770040 -
Flags: review?(bzbarsky)
Comment 12•11 years ago
|
||
Comment on attachment 770040 [details] [diff] [review] patch r=me. Good catch! Please file a followup bug on the assertions that get hit here?
Attachment #770040 -
Flags: review?(bzbarsky) → review+
Updated•11 years ago
|
Flags: needinfo?(hsivonen)
Assignee | ||
Comment 13•11 years ago
|
||
> Please file a followup bug on the assertions that get hit here?
With this patch we don't have this assertions. We don't need a followup.
Keywords: checkin-needed
Assignee | ||
Comment 15•11 years ago
|
||
Comment on attachment 770040 [details] [diff] [review] patch [Security approval request comment] How easily could an exploit be constructed based on the patch? I don't know. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? Yes. Which older supported branches are affected by this flaw? this is caused by bug 870787. So probably just the nightly. If not all supported branches, which bug introduced the flaw? bug 870787 How likely is this patch to cause regressions; how much testing does it need? No regressions can be introduced by this patch.
Attachment #770040 -
Flags: sec-approval?
Comment 16•11 years ago
|
||
Comment on attachment 770040 [details] [diff] [review] patch sec-approval+ for trunk. This is also marked as affecting aurora so you should prepare an aurora patch and nominate it after this gets into trunk. This will keep us from shipping this issue.
Attachment #770040 -
Flags: sec-approval? → sec-approval+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 17•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/9c76c08e6437 Be sure to land a test for this at some point.
Flags: in-testsuite?
Keywords: checkin-needed
Comment 18•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9c76c08e6437
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Updated•11 years ago
|
Flags: sec-bounty? → sec-bounty+
Updated•11 years ago
|
Comment 20•11 years ago
|
||
needinfo :baku to help with the needed patch for Fx24(our current beta).
Flags: needinfo?(amarchesini)
Assignee | ||
Comment 21•11 years ago
|
||
Comment on attachment 770040 [details] [diff] [review] patch [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 870787 User impact if declined: a crash can happen. Risk to taking this patch (and alternatives if risky): None. This patch adds a simple 'if' condition. String or IDL/UUID changes made by this patch: no
Attachment #770040 -
Flags: approval-mozilla-beta?
Updated•11 years ago
|
Attachment #770040 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•11 years ago
|
Keywords: checkin-needed
Updated•11 years ago
|
Flags: needinfo?(amarchesini)
Comment 22•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/58b6029c8aea
Keywords: checkin-needed
Updated•11 years ago
|
Whiteboard: [asan] → [asan][adv-main24-]
Updated•11 years ago
|
Comment 23•11 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM UTC-4][PTO 10/26 - 11/3] from comment #17) > https://hg.mozilla.org/integration/mozilla-inbound/rev/9c76c08e6437 > > Be sure to land a test for this at some point. Anyone?
Updated•10 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•