Bug 682727 (CVE-2011-3652)

Crash during datamove (ACCESS VIOLATION READ) [@ memcpy | nsAString_internal::Assign(unsigned short const*, unsigned int)]

RESOLVED FIXED in Firefox 8

Status

()

Core
Networking: Cache
--
critical
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: Rh0, Assigned: michal)

Tracking

Trunk
mozilla10
x86
Windows XP
Points:
---
Bug Flags:
sec-bounty +

Firefox Tracking Flags

(firefox6- wontfix, firefox7- wontfix, firefox8+ fixed, firefox9+ fixed, firefox10+ fixed, status1.9.2 unaffected)

Details

(Whiteboard: [sg:critical][oom][qa?], crash signature)

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Created attachment 556407 [details]
testcase to crash Firefox 6

When running the attached testcase on a native WinXP SP3 installation (not in VirtualBox)with Firefox 6, it crashes during a datamove at memcpy in method nsAString_internal::Assign(unsigned short const*, unsigned int).

Crash IDs:
bp-61c8c0ae-0b67-4063-bd2b-906392110828
bp-5534e024-7733-42bd-a565-464ce2110828

WinDBG states this as Probably Exploitable. This memcpy crash seems similar to the one in bug 505305 ([sg:critical?]). The testcase is based on the testcase of bug 651990.

Steps to reproduce:
1. Open testcase in Fx6
2. Click the button

Greetings, 
Rh0
(Reporter)

Updated

6 years ago
Crash Signature: bp-61c8c0ae-0b67-4063-bd2b-906392110828 bp-5534e024-7733-42bd-a565-464ce2110828

Updated

6 years ago
Crash Signature: bp-61c8c0ae-0b67-4063-bd2b-906392110828 bp-5534e024-7733-42bd-a565-464ce2110828 → [@ memcpy | nsAString_internal::Assign(unsigned short const*, unsigned int) ]

Updated

6 years ago
Attachment #556407 - Attachment mime type: text/plain → text/html

Comment 1

6 years ago
WinXP w/1G ram

Debug Nightly/Aurora/Beta crashed but MSVC was unable to capture the crash.

Beta/7
bp-8541ebe3-25e1-4b5e-896c-59ea42110828 (no signature)

Aurora/8
bp-731b301b-ada4-45ff-9bd8-2af542110828
[@ nsAString_internal::Assign(nsAString_internal const&) ]

Nightly/9
bp-66d68216-8e2e-4b84-8a47-cdcce2110828
[@ memcpy | nsAString_internal::Assign(wchar_t const*, unsigned int) ]
Status: UNCONFIRMED → NEW
status-firefox6: --- → affected
status-firefox7: --- → affected
status-firefox8: --- → affected
status-firefox9: --- → affected
Ever confirmed: true
Version: 6 Branch → Trunk

Updated

6 years ago
Component: General → Networking: Cache
Product: Firefox → Core
QA Contact: general → networking.cache
Whiteboard: [sg:critical?]
Not realistic to squeeze into Firefox 7, targeting the next release.
Assignee: nobody → michal.novotny
tracking-firefox6: --- → -
tracking-firefox7: --- → -
tracking-firefox8: --- → +
tracking-firefox9: --- → +

Updated

6 years ago
Whiteboard: [sg:critical?] → [sg:critical]
(Assignee)

Comment 4

6 years ago
Created attachment 563014 [details] [diff] [review]
fix

The problem is that the allocation of the buffer fails while flattening the string at http://hg.mozilla.org/mozilla-central/annotate/7f4867717226/js/src/vm/String.cpp#l207 but we don't set mValid=false in xpc_qsDOMString/xpc_qsAUTF8String.
Attachment #563014 - Flags: review?(jst)
(Assignee)

Comment 5

6 years ago
Pushed to tryserver https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=d78227785476
Comment on attachment 563014 [details] [diff] [review]
fix

Good find, Michal! r=jst
Attachment #563014 - Flags: review?(jst) → review+
(Assignee)

Updated

6 years ago
Attachment #563014 - Flags: superreview?(joshmoz)

Comment 7

6 years ago
Comment on attachment 563014 [details] [diff] [review]
fix

>     JSString *s = InitOrStringify<traits>(cx, v, pval, nullBehavior,
>                                           undefinedBehavior);
>     if (!s)
>         return;
...
>     JSString *s = InitOrStringify<traits>(cx, v, pval, eNull, eNull);
>     if (!s)
>         return;

I don't know this code at all but I want to make sure we don't also need to set mValid to JS_FALSE in the early returns prior to the ones you patched.
InitOrStringify() already set mValid to false in all error cases, so don't need to set it again in those cases.

Updated

6 years ago
Attachment #563014 - Flags: superreview?(joshmoz) → superreview+
(Assignee)

Comment 9

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/dd0cc50e9fe6
Whiteboard: [sg:critical] → [sg:critical] [inbound]
Target Milestone: --- → mozilla10

Comment 10

6 years ago
http://hg.mozilla.org/mozilla-central/rev/dd0cc50e9fe6
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical] [inbound] → [sg:critical][oom]

Updated

6 years ago
Attachment #563014 - Flags: approval-mozilla-beta?
Attachment #563014 - Flags: approval-mozilla-aurora?
I tried verifying this in a Win XP VM and on a Win 7 machine using Mozilla/5.0 (Windows NT 6.1; rv:10.0a1) Gecko/20111006 Firefox/10.0a1, but in both cases Firefox stops responding but no crash reporter comes up.
Comment on attachment 563014 [details] [diff] [review]
fix

Approved for Aurora (Update 9) and Beta (Update 8).  Please land as soon as appropriate.
Attachment #563014 - Flags: approval-mozilla-beta?
Attachment #563014 - Flags: approval-mozilla-beta+
Attachment #563014 - Flags: approval-mozilla-aurora?
Attachment #563014 - Flags: approval-mozilla-aurora+
(Reporter)

Comment 13

6 years ago
To comment 11 :
Using XP and Firefox 6 in my testings, the crash appeared only on native installations of XP and not in VMs running in VirtualBox.
I used both a VM and real hardware to test, and got the same result - the nightly stopped responding. So it does not seem that the patch really fixes the issue unless I am misunderstanding something.

(In reply to Rh0 from comment #13)
> To comment 11 :
> Using XP and Firefox 6 in my testings, the crash appeared only on native
> installations of XP and not in VMs running in VirtualBox.
I don't have access to Win hardware at the moment, so I would prefer if we could hold off checking this in until QA can do some more testing on Windows hardware in the lab. Thanks.
I tried reproducing this problem on Fx5, 7.0.1, and the most recent Nightly on a Win7 machine. In all three the testcase crashes the application. It takes a while, but they all crash.

When I try to look at the crash reports, they can't be found: bp-1dbea091-1abf-448b-8fbb-313ad2111006

It could be that this is now crashing in a different place, but without seeing the crash signature I can't tell. I'll re-open as soon as the crash report shows up, or after consulting with someone in the security team.
I was able to crash again, but I get a different signature: 
https://crash-stats.mozilla.com/report/index/bp-29f53417-90cb-44c6-9459-c185f2111007

Should I file new bug for this crash and should it be security sensitive?
(Assignee)

Comment 18

6 years ago
(In reply to Marcia Knous [:marcia] from comment #17)
> I was able to crash again, but I get a different signature: 
> https://crash-stats.mozilla.com/report/index/bp-29f53417-90cb-44c6-9459-
> c185f2111007
> 
> Should I file new bug for this crash and should it be security sensitive?

No, this is an intentional crash in OOM situation.
(Reporter)

Comment 19

6 years ago
(In reply to juan becerra [:juanb] from comment #16)
> I tried reproducing this problem on Fx5, 7.0.1, and the most recent Nightly
> on a Win7 machine. In all three the testcase crashes the application. It
> takes a while, but they all crash.
> 
> When I try to look at the crash reports, they can't be found:
> bp-1dbea091-1abf-448b-8fbb-313ad2111006
> 
> It could be that this is now crashing in a different place, but without
> seeing the crash signature I can't tell. I'll re-open as soon as the crash
> report shows up, or after consulting with someone in the security team.

With Fx7.0.1 in WinXP/2GB RAM running in the latest VirtualBox (4.1.4), I get the crash at memcpy:
bp-4ec14c4f-b56e-4e44-b194-520422111007

but also another one ( [@ js_NextActiveContext(JSRuntime*, JSContext*)] ):
bp-6015acf5-0c04-467a-88ad-365f82111007

Fx10.0a1 crashes as well, but I cannot get a signature, too:
bp-3f3d522c-282d-4f15-a872-f07152111007

In this case Firefox can be started in WinDBG (Windows Debugger) to check the crash. In my case I get for Fx10.0a1 this exception:

(f60.e58): Break instruction exception - code 80000003 (first chance)
eax=00000000 ebx=7d929740 ecx=7817a4c8 edx=781c3c70 esi=7813e457 edi=7817a392
eip=00a8193d esp=001275b8 ebp=7f9f9470 iopl=0         nv up ei pl nz na pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00000206
mozalloc!mozalloc_abort+0x2b:
00a8193d cc              int     3

And this call stack (obtained with command "kc 10" in WinDBG):

mozalloc!mozalloc_abort
mozalloc!mozalloc_handle_oom
xul!nsTArray_base<nsTArrayDefaultAllocator>::EnsureCapacity
xul!gfxTextRun::CopyGlyphDataFrom
xul!TextRunWordCache::LookupWord
xul!TextRunWordCache::MakeTextRun
xul!MakeTextRun
xul!BuildTextRunsScanner::BuildTextRunForFrames
xul!BuildTextRunsScanner::FlushFrames
xul!BuildTextRuns
xul!nsIFrame::InvalidateInternal
xul!nsIFrame::InvalidateInternal
xul!nsIFrame::InvalidateInternal
xul!nsBlockFrame::InvalidateInternal
xul!nsLayoutUtils::IntrinsicForContainer
xul!nsTableCellFrame::GetMinWidth


I think this a crash/break on purpose due to OOM.
https://hg.mozilla.org/releases/mozilla-aurora/rev/433458808fbe
https://hg.mozilla.org/releases/mozilla-beta/rev/890a66b91702
status-firefox10: --- → fixed
status-firefox6: affected → wontfix
status-firefox7: affected → wontfix
status-firefox8: affected → fixed
status-firefox9: affected → fixed
tracking-firefox10: --- → +
Is the attached testcase only usable in a debug build? Can QA verify this fix on a "normal" build?
Whiteboard: [sg:critical][oom] → [sg:critical][oom][qa?]
(Assignee)

Comment 22

6 years ago
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #21)
> Is the attached testcase only usable in a debug build? Can QA verify this
> fix on a "normal" build?

Yes, and even with debug build the crash is not 100% reproducible. It depends on what is in the uninitialized buffer and if GC runs during the test.
I assume "yes" refers to whether or not this needs a debug build. Based on comment 22, I don't see how QA will be able to reliable verify this fix.
(Assignee)

Comment 24

6 years ago
Branch 1.9.2 isn't affected and doesn't need this patch. Member variable mValid seems to be set correctly in all 3 classes (xpc_qsDOMString, xpc_qsAString, xpc_qsACString).
status1.9.2: --- → unaffected
Alias: CVE-2011-3652
Group: core-security
Flags: sec-bounty+
You need to log in before you can comment on or make changes to this bug.