733031 - OOM Crash [@ nsMemory::Clone] with possible integer overflow

Status: RESOLVED WORKSFORME

(Core :: Preferences: Backend, defect)

Description

[Christian Holler (:decoder)]

Tested on m-c revision 8ea5c983743f: During OOM testing I got the following crash:

Program received signal SIGSEGV, Segmentation fault.
0x00002aaaad2464a1 in nsMemory::Clone (ptr=0x2aaaae464cd6, size=4294967267) at /usr/include/x86_64-linux-gnu/bits/string3.h:52
52        return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));
#0  0x00002aaaad2464a1 in nsMemory::Clone (ptr=0x2aaaae464cd6, size=4294967267) at /usr/include/x86_64-linux-gnu/bits/string3.h:52
#1  0x00002aaaac5d6f41 in nsPrefBranch::GetChildList (this=0x26dc920, aStartingAt=<optimized out>, aCount=0x7fffffff8448, aChildArray=0x7fffffff8460) at /srv/repos/browser/mozilla-central/modules/libpref/src/nsPrefBranch.cpp:549
#2  0x00002aaaad29f326 in NS_InvokeByIndex_P (that=<optimized out>, methodIndex=<optimized out>, paramCount=3, params=<optimized out>) at /srv/repos/browser/mozilla-central/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:195
#3  0x00002aaaacde51a2 in Invoke (this=0x7fffffff83f0) at /srv/repos/browser/mozilla-central/js/xpconnect/src/XPCWrappedNative.cpp:2881
#4  CallMethodHelper::Call (this=0x7fffffff83f0) at /srv/repos/browser/mozilla-central/js/xpconnect/src/XPCWrappedNative.cpp:2212
#5  0x00002aaaacde586b in XPCWrappedNative::CallMethod (ccx=..., mode=<optimized out>) at /srv/repos/browser/mozilla-central/js/xpconnect/src/XPCWrappedNative.cpp:2178
#6  0x00002aaaacdeb4c4 in XPC_WN_CallMethod (cx=0xd25a10, argc=1, vp=0x2aaabacc42d8) at /srv/repos/browser/mozilla-central/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1540
#7  0x00002aaaad5fc13d in js::CallJSNative (cx=0xd25a10, native=0x2aaaacdeb367 <XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*)>, args=...) at /srv/repos/browser/mozilla-central/js/src/jscntxtinlines.h:311
=> 0x2aaaad2464a1 <nsMemory::Clone(void const*, unsigned long)+35>:     rep movsb %ds:(%rsi),%es:(%rdi)
rsi            0x2aaaae4b3000   46912556969984
rdi            0x2aaac404e33a   46912921461562
ds             0x0      0
es             0x0      0

From inspecting this code piece

548           outArray[dwIndex] = (char *)nsMemory::Clone(
549             element.get() + mPrefRootLength, element.Length() - mPrefRootLength + 1);

it's likely that element.Length() is 0 (cannot check as the value is optimized out), but mPrefRootLength is 30 which fits with size=4294967267 in the ::Clone call (2^32 - 30 + 1).


The last allocation failure trace was this (but possibly, an earlier trace could also be the cause):

#0 /srv/repos/browser/mozilla-central/objdir-ff-gcc64dbg/dist/bin/libmozalloc.so(moz_malloc+0x5f)
#1 nsStringBuffer::Alloc(unsigned long) at xpcom/string/src/nsSubstring.cpp:210
#2 nsACString_internal::MutatePrep(unsigned int, char**, unsigned int*) at xpcom/string/src/nsTSubstring.cpp:163
#3 nsACString_internal::ReplacePrepInternal(unsigned int, unsigned int, unsigned int, unsigned int) at xpcom/string/src/nsTSubstring.cpp:199
#4 nsACString_internal::Assign(char const*, unsigned int) at xpcom/string/src/nsTSubstring.cpp:335
#5 nsTArray_base<nsTArrayDefaultAllocator>::IncrementLength(unsigned int) at objdir-ff-gcc64dbg/modules/libpref/src/../../../dist/include/nsTArray.h:277
#6 PL_DHashTableEnumerate at objdir-ff-gcc64dbg/xpcom/build/pldhash.cpp:755
#7 nsTArray_base<nsTArrayDefaultAllocator>::Length() const at objdir-ff-gcc64dbg/modules/libpref/src/../../../dist/include/nsTArray.h:224
#8 /srv/repos/browser/mozilla-central/objdir-ff-gcc64dbg/dist/bin/libxul.so(NS_InvokeByIndex_P+0x23a)
#9 CallMethodHelper::Invoke() at js/xpconnect/src/XPCWrappedNative.cpp:2881
#10 XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) at js/xpconnect/src/XPCWrappedNative.cpp:2178
#11 XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) at js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1540
#12 CallJSNative at js/src/jscntxtinlines.h:311
#13 js::InvokeKernel(JSContext*, js::CallArgs, js::MaybeConstruct) at js/src/jsinterp.cpp:499
#14 js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) at js/src/jsinterp.cpp:2699
#15 js::StackFrame::returnValue() at js/src/vm/Stack.h:1003

This testing run focused on all callers that have frames like #4 (or #5), which probably accounts for a lot of callers. If you think this allocation trace is unrelated, let me know and I will record all allocation traces so we can find the right one.

Leaving component unassigned, as I don't know where the cause for this is located.

Comment 1

[Benjamin Smedberg]

Attachment: Check for string assignment failure, rev. 1

This sucks, and we should really just fix it by making strings infallible by default, but this would also fix the problem. I think that if we can just fix the strings in a reasonble timeframe (just posted to dev.platform, I think I can do it this cycle) that we shouldn't take this patch except perhaps on ESR, and even then this would be fabulously difficult to exploit, probably impossible.

Comment 2

[Benjamin Smedberg]

Given that this is an UMR from well-controlled memory and any possible result is available only to chrome code, I don't think that this is a security issue and we should open it. khuey do you agree?

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #2)
> Given that this is an UMR from well-controlled memory and any possible
> result is available only to chrome code, I don't think that this is a
> security issue and we should open it. khuey do you agree?

I agree.

Comment 4

[Alex Keybl [:akeybl]]

Since this is no longer a security issue, and isn't a particularly critical crash, please re-nominate if there's another reason to track for release.

Comment 5

[Benjamin Smedberg]

Strings are infallible now.