Closed
Bug 1172055
(CVE-2015-7174)
Opened 8 years ago
Closed 8 years ago
Overflow in nsAttrAndChildArray::GrowBy causes memory-safety bug
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla42
People
(Reporter: q1, Assigned: baku)
Details
(Keywords: csectype-intoverflow, sec-moderate, Whiteboard: [post-critsmash-triage][adv-main41+][adv-esr38.3+])
Attachments
(1 file, 1 obsolete file)
2.80 KB,
patch
|
ritu
:
approval-mozilla-beta+
ritu
:
approval-mozilla-esr38+
mpotharaju
:
approval-mozilla-b2g37+
mpotharaju
:
approval‑mozilla‑b2g37_v2_2r+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:36.0) Gecko/20100101 Firefox/36.0 Build ID: 20150305021524 Steps to reproduce: nsAttrAndChildArray::GrowBy (38.0.1\dom\base\nsAttrAndChildArray.cpp) can overflow if mBufferSize becomes very large, causing a realloc to a tiny buffer size. Other code in the class subsequently writes to unallocated space, potentially permitting the execution of attacker-chosen code, as by modifying an object's virtual-function table: 763: bool 764: nsAttrAndChildArray::GrowBy(uint32_t aGrowSize) 765: { 766: uint32_t size = mImpl ? mImpl->mBufferSize + NS_IMPL_EXTRA_SIZE : 0; 767: uint32_t minSize = size + aGrowSize; 768: 769: if (minSize <= ATTRCHILD_ARRAY_LINEAR_THRESHOLD) { 770: do { 771: size += ATTRCHILD_ARRAY_GROWSIZE; 772: } while (size < minSize); 773: } 774: else { 775: size = 1u << mozilla::CeilingLog2(minSize); 776: } 777: 778: bool needToInitialize = !mImpl; 779: Impl* newImpl = static_cast<Impl*>(moz_realloc(mImpl, size * sizeof(void*))); 780: ... If size (from mImpl->mBufferSize) is very close to 0xffffffff, line 767 can overflow. If the condition on line 769 is satisfied, size also can overflow. If the condition on line 767 is not satisfied, line 775 computes a tiny size from minSize. In either case, line 779 allocates a buffer far too small to contain all the existing data, and other code in the class writes to the unallocated area.
Updated•8 years ago
|
Flags: sec-bounty?
Updated•8 years ago
|
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #8624155 -
Flags: review?(ehsan)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → amarchesini
Updated•8 years ago
|
Attachment #8624155 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 2•8 years ago
|
||
Comment on attachment 8624155 [details] [diff] [review] crash6.patch [Security approval request comment] How easily could an exploit be constructed based on the patch? Quite hard. the string has to be 2^32 bytes. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? yes. we check the size of the growing string. 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? Easy to create backport patches. How likely is this patch to cause regressions; how much testing does it need? No regressions.
Attachment #8624155 -
Flags: sec-approval?
Updated•8 years ago
|
Keywords: sec-moderate
Comment 3•8 years ago
|
||
Comment on attachment 8624155 [details] [diff] [review] crash6.patch Clearing sec-approval since this has been rated a sec-moderate and you don't need approval to check those in.
Attachment #8624155 -
Flags: sec-approval?
Assignee | ||
Comment 4•8 years ago
|
||
Attachment #8624155 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 5•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a9e266811633
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a9e266811633
Status: UNCONFIRMED → RESOLVED
Closed: 8 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Version: 38 Branch → 42 Branch
Updated•8 years ago
|
Flags: sec-bounty? → sec-bounty+
Comment 7•8 years ago
|
||
Please nominate this for Beta/esr38/b2g37 approval.
status-b2g-v2.0:
--- → wontfix
status-b2g-v2.0M:
--- → wontfix
status-b2g-v2.1:
--- → wontfix
status-b2g-v2.1S:
--- → wontfix
status-b2g-v2.2:
--- → affected
status-b2g-v2.2r:
--- → affected
status-b2g-master:
--- → fixed
status-firefox40:
--- → wontfix
status-firefox41:
--- → affected
status-firefox-esr38:
--- → affected
Flags: needinfo?(amarchesini)
Target Milestone: --- → mozilla42
Assignee | ||
Comment 8•8 years ago
|
||
Comment on attachment 8631875 [details] [diff] [review] 252140.diff [Approval Request Comment] Bug caused by (feature/regressing bug #): Quite old feature/code. User impact if declined: a crash *could* happen. Testing completed: none Risk to taking this patch (and alternatives if risky): not too much. We just check the size with CeckedUint32 before allocating. String or UUID changes made by this patch: none [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: Fix Landed on Version: 42? Risk to taking this patch (and alternatives if risky): none String or UUID changes made by this patch: none
Flags: needinfo?(amarchesini)
Attachment #8631875 -
Flags: approval-mozilla-esr38?
Attachment #8631875 -
Flags: approval-mozilla-beta?
Attachment #8631875 -
Flags: approval-mozilla-b2g37?
Comment on attachment 8631875 [details] [diff] [review] 252140.diff Approving for uplift to Beta41 and esr38.3.0. This is a sec fix that has been in Nightly and Aurora for about 6 weeks. It seems safe to uplift.
Attachment #8631875 -
Flags: approval-mozilla-esr38?
Attachment #8631875 -
Flags: approval-mozilla-esr38+
Attachment #8631875 -
Flags: approval-mozilla-beta?
Attachment #8631875 -
Flags: approval-mozilla-beta+
Mahe, n-i'ing you on the b2g uplift request. Not sure if you do it or somebody else.
Flags: needinfo?(mpotharaju)
Comment 12•8 years ago
|
||
Comment on attachment 8631875 [details] [diff] [review] 252140.diff Ryan, Please uplift this to b2g-v2.2, b2g-v2.2r, b2g-37 Thanks
Flags: needinfo?(mpotharaju)
Attachment #8631875 -
Flags: approval‑mozilla‑b2g37_v2_2r+
Attachment #8631875 -
Flags: approval-mozilla-b2g37?
Attachment #8631875 -
Flags: approval-mozilla-b2g37+
Comment 14•8 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c5295e651b78 https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/c5295e651b78 https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/71aa93494a06
Comment 15•8 years ago
|
||
Fix for non-unified bustage. I'll push it to other branches as a ride-along as well. https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c03e2bc6a3a4 https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/c03e2bc6a3a4 https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/d058b8819614
Comment 17•8 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/32f4ac616613 https://hg.mozilla.org/releases/mozilla-beta/rev/bacde2f7a48b
Updated•8 years ago
|
Group: core-security → core-security-release
tracking-firefox-esr38:
--- → 41+
Updated•8 years ago
|
Whiteboard: [post-critsmash-triage]
Updated•8 years ago
|
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main41+]
Updated•8 years ago
|
Whiteboard: [post-critsmash-triage][adv-main41+] → [post-critsmash-triage][adv-main41+][adv-esr38.3+]
Updated•8 years ago
|
Alias: CVE-2015-7174
Updated•7 years ago
|
Group: core-security-release
Updated•4 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•