All users were logged out of Bugzilla on October 13th, 2018
Bug 1172055 (CVE-2015-7174)

Overflow in nsAttrAndChildArray::GrowBy causes memory-safety bug

RESOLVED FIXED in Firefox 41, Firefox OS v2.1S

Status

()

RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: q1, Assigned: baku)

Tracking

({csectype-intoverflow, sec-moderate})

42 Branch
mozilla42
csectype-intoverflow, sec-moderate
Points:
---
Bug Flags:
sec-bounty +

Firefox Tracking Flags

(firefox40 wontfix, firefox41 fixed, firefox42 fixed, firefox-esr3841+ fixed, b2g-v2.0 wontfix, b2g-v2.0M wontfix, b2g-v2.1 wontfix, b2g-v2.1S fixed, b2g-v2.2 fixed, b2g-v2.2r fixed, b2g-master fixed)

Details

(Whiteboard: [post-critsmash-triage][adv-main41+][adv-esr38.3+])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
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.
Flags: sec-bounty?
Component: Untriaged → DOM
Keywords: csectype-intoverflow
Product: Firefox → Core
(Assignee)

Comment 1

3 years ago
Created attachment 8624155 [details] [diff] [review]
crash6.patch
Attachment #8624155 - Flags: review?(ehsan)
(Assignee)

Updated

3 years ago
Assignee: nobody → amarchesini

Updated

3 years ago
Attachment #8624155 - Flags: review?(ehsan) → review+
(Assignee)

Comment 2

3 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?
Keywords: sec-moderate
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

3 years ago
Created attachment 8631875 [details] [diff] [review]
252140.diff
Attachment #8624155 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a9e266811633
Status: UNCONFIRMED → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Version: 38 Branch → 42 Branch
Flags: sec-bounty? → sec-bounty+
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

3 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 9

3 years ago
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 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+

Updated

3 years ago
Group: core-security → core-security-release
tracking-firefox-esr38: --- → 41+
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main41+]
Whiteboard: [post-critsmash-triage][adv-main41+] → [post-critsmash-triage][adv-main41+][adv-esr38.3+]
Alias: CVE-2015-7174
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.