Closed Bug 1645415 Opened 4 years ago Closed 4 years ago

SUMMARY: AddressSanitizer: heap-use-after-free /builds/worker/checkouts/gecko/js/src/builtin/String.cpp:3500:31 in SplitSingleCharHelper<unsigned char>

Categories

(Core :: JavaScript: GC, defect, P1)

Product:
Core
Component:
JavaScript: GC
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla80
Tracking Flags:
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox77 --- unaffected
firefox78 --- unaffected
firefox79 + fixed
firefox80 + fixed

People

(Reporter: nataliaCs, Assigned: sfink)

References

(Regression)

Details

(4 keywords, Whiteboard: [sec-survey][post-critsmash-triage])

Attachments

(1 file)

Bug 1645415 - Mark all relevant strings non-deduplicatable
47 bytes, text/x-phabricator-request
tjr
: approval-mozilla-beta+
tjr
: sec-approval+
Details | Review

Push: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=b3831bd0c9c8e8744993a0d5be6797b942f39662&selectedTaskRun=TBqgwhxdRbexeEYbe_S8fg.0&searchStr=Linux%2C18.04%2Cx64%2Casan%2Copt%2CMochitests%2Cwithout%2Ce10s%2Ctest-linux1804-64-asan%2Fopt-mochitest-chrome-1proc-2%2CM-1proc%28c2%29

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=306094649&repo=try&lineNumber=6100

[task 2020-06-12T12:05:00.164Z] 12:05:00 INFO - TEST-START | dom/tests/mochitest/webcomponents/test_custom_element_htmlconstructor_chrome.html
[task 2020-06-12T12:05:01.555Z] 12:05:01 INFO - =================================================================
[task 2020-06-12T12:05:01.555Z] 12:05:01 ERROR - ==4720==ERROR: AddressSanitizer: heap-use-after-free on address 0x60600007e7b1 at pc 0x7fef40f9205b bp 0x7ffc1dcfb650 sp 0x7ffc1dcfb648
[task 2020-06-12T12:05:01.555Z] 12:05:01 INFO - READ of size 1 at 0x60600007e7b1 thread T0
[task 2020-06-12T12:05:01.961Z] 12:05:01 INFO - #0 0x7fef40f9205a in SplitSingleCharHelper<unsigned char> /builds/worker/checkouts/gecko/js/src/builtin/String.cpp:3500:31
[task 2020-06-12T12:05:01.961Z] 12:05:01 INFO - #1 0x7fef40f9205a in SplitSingleCharHelper /builds/worker/checkouts/gecko/js/src/builtin/String.cpp:3535:12
[task 2020-06-12T12:05:01.961Z] 12:05:01 INFO - #2 0x7fef40f9205a in js::StringSplitString(JSContext*, JS::Handle<js::ObjectGroup*>, JS::Handle<JSString*>, JS::Handle<JSString*>, unsigned int) /builds/worker/checkouts/gecko/js/src/builtin/String.cpp:3565:12
[task 2020-06-12T12:05:01.977Z] 12:05:01 INFO - #3 0x7fef4120e1f5 in js::intrinsic_StringSplitString(JSContext*, unsigned int, JS::Value*) /builds/worker/checkouts/gecko/js/src/vm/SelfHosting.cpp:1617:20
[task 2020-06-12T12:05:01.984Z] 12:05:01 INFO - #4 0x3919f9b5cf7f (<unknown module>)
[task 2020-06-12T12:05:01.985Z] 12:05:01 INFO - 0x60600007e7b1 is located 17 bytes inside of 56-byte region [0x60600007e7a0,0x60600007e7d8)
[task 2020-06-12T12:05:01.985Z] 12:05:01 INFO - freed by thread T8 (JS Helper) here:
[task 2020-06-12T12:05:01.987Z] 12:05:01 INFO - #0 0x558e09eb909d in free /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:123:3
[task 2020-06-12T12:05:02.010Z] 12:05:02 INFO - #1 0x7fef418a1f94 in js_free /builds/worker/workspace/obj-build/dist/include/js/Utility.h:431:3
[task 2020-06-12T12:05:02.011Z] 12:05:02 INFO - #2 0x7fef418a1f94 in freeUntracked /builds/worker/checkouts/gecko/js/src/gc/FreeOp.h:75:33
[task 2020-06-12T12:05:02.011Z] 12:05:02 INFO - #3 0x7fef418a1f94 in js::gc::GCRuntime::freeFromBackgroundThread(js::AutoLockHelperThreadState&) /builds/worker/checkouts/gecko/js/src/gc/GC.cpp:3469:12
[task 2020-06-12T12:05:02.012Z] 12:05:02 INFO - #4 0x7fef418a1b09 in js::gc::BackgroundFreeTask::run() /builds/worker/checkouts/gecko/js/src/gc/GC.cpp:3445:7
[task 2020-06-12T12:05:02.013Z] 12:05:02 INFO - #5 0x7fef418cfb16 in js::GCParallelTask::runTask() /builds/worker/checkouts/gecko/js/src/gc/GCParallelTask.cpp:146:3
[task 2020-06-12T12:05:02.014Z] 12:05:02 INFO - #6 0x7fef418cf8f5 in js::GCParallelTask::runFromHelperThread(js::AutoLockHelperThreadState&) /builds/worker/checkouts/gecko/js/src/gc/GCParallelTask.cpp:131:5
[task 2020-06-12T12:05:02.015Z] 12:05:02 INFO - previously allocated by thread T0 here:
[task 2020-06-12T12:05:02.015Z] 12:05:02 INFO - #0 0x558e09eb931d in malloc /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:145:3
[task 2020-06-12T12:05:02.016Z] 12:05:02 INFO - #1 0x7fef40e305c0 in js_arena_malloc /builds/worker/workspace/obj-build/dist/include/js/Utility.h:384:10
[task 2020-06-12T12:05:02.017Z] 12:05:02 INFO - #2 0x7fef40e305c0 in js_pod_arena_malloc<unsigned char> /builds/worker/workspace/obj-build/dist/include/js/Utility.h:592:26
[task 2020-06-12T12:05:02.018Z] 12:05:02 INFO - #3 0x7fef40e305c0 in maybe_pod_arena_malloc<unsigned char> /builds/worker/workspace/obj-build/dist/include/js/AllocPolicy.h:31:12
[task 2020-06-12T12:05:02.018Z] 12:05:02 INFO - #4 0x7fef40e305c0 in pod_arena_malloc<unsigned char> /builds/worker/workspace/obj-build/dist/include/js/AllocPolicy.h:131:18
[task 2020-06-12T12:05:02.019Z] 12:05:02 INFO - #5 0x7fef40e305c0 in pod_malloc<unsigned char> /builds/worker/checkouts/gecko/js/src/util/StringBuffer.h:42:18
[task 2020-06-12T12:05:02.020Z] 12:05:02 INFO - #6 0x7fef40e305c0 in mozilla::Vector<unsigned char, 64ul, js::StringBufferAllocPolicy>::extractOrCopyRawBuffer() /builds/worker/workspace/obj-build/dist/include/mozilla/Vector.h:1454:28
[task 2020-06-12T12:05:02.021Z] 12:05:02 INFO - #7 0x7fef40e2e32e in ExtractWellSized<unsigned char, mozilla::Vector<unsigned char, 64, js::StringBufferAllocPolicy> > /builds/worker/checkouts/gecko/js/src/util/StringBuffer.cpp:26:19
[task 2020-06-12T12:05:02.022Z] 12:05:02 INFO - #8 0x7fef40e2e32e in JSLinearString* js::StringBuffer::finishStringInternal<unsigned char>(JSContext*) /builds/worker/checkouts/gecko/js/src/util/StringBuffer.cpp:91:7
[task 2020-06-12T12:05:02.029Z] 12:05:02 INFO - #9 0x7fef40c5deed in js::array_join(JSContext*, unsigned int, JS::Value*) /builds/worker/checkouts/gecko/js/src/builtin/Array.cpp:1511:22
[task 2020-06-12T12:05:02.037Z] 12:05:02 INFO - #10 0x3919f9b5cf7f (<unknown module>)
[task 2020-06-12T12:05:02.038Z] 12:05:02 INFO - Thread T8 (JS Helper) created by T0 here:
[task 2020-06-12T12:05:02.039Z] 12:05:02 INFO - #0 0x558e09ea3aca in pthread_create /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:209:3
[task 2020-06-12T12:05:02.040Z] 12:05:02 INFO - #1 0x7fef40e2b0c1 in js::Thread::create(void* ()(void), void*) /builds/worker/checkouts/gecko/js/src/threading/posix/PosixThread.cpp:52:7
[task 2020-06-12T12:05:02.056Z] 12:05:02 INFO - #2 0x7fef40f3da94 in bool js::Thread::init<void (&)(void*), js::HelperThread*>(void (&)(void*), js::HelperThread*&&) /builds/worker/checkouts/gecko/js/src/threading/Thread.h:90:12
[task 2020-06-12T12:05:02.058Z] 12:05:02 INFO - #3 0x7fef40f361fc in js::GlobalHelperThreadState::ensureInitialized() /builds/worker/checkouts/gecko/js/src/vm/HelperThreads.cpp:1160:27
[task 2020-06-12T12:05:02.059Z] 12:05:02 INFO - #4 0x7fef411cdbfc in JSRuntime::init(JSContext*, unsigned int) /builds/worker/checkouts/gecko/js/src/vm/Runtime.cpp:200:32
[task 2020-06-12T12:05:02.081Z] 12:05:02 INFO - #5 0x7fef41056c9c in js::NewContext(unsigned int, JSRuntime*) /builds/worker/checkouts/gecko/js/src/vm/JSContext.cpp:182:17
[task 2020-06-12T12:05:02.088Z] 12:05:02 INFO - #6 0x7fef362100a2 in mozilla::CycleCollectedJSContext::Initialize(JSRuntime*, unsigned int) /builds/worker/checkouts/gecko/xpcom/base/CycleCollectedJSContext.cpp:123:16
[task 2020-06-12T12:05:02.104Z] 12:05:02 INFO - #7 0x7fef37cb162f in XPCJSContext::Initialize() /builds/worker/checkouts/gecko/js/xpconnect/src/XPCJSContext.cpp:1133:32
[task 2020-06-12T12:05:02.104Z] 12:05:02 INFO - #8 0x7fef37cb2df2 in XPCJSContext::NewXPCJSContext() /builds/worker/checkouts/gecko/js/xpconnect/src/XPCJSContext.cpp:1329:23
[task 2020-06-12T12:05:02.112Z] 12:05:02 INFO - #9 0x7fef37d41858 in nsXPConnect::InitJSContext() /builds/worker/checkouts/gecko/js/xpconnect/src/nsXPConnect.cpp:80:25
[task 2020-06-12T12:05:02.112Z] 12:05:02 INFO - #10 0x7fef36476ccd in NS_InitXPCOM /builds/worker/checkouts/gecko/xpcom/build/XPCOMInit.cpp:491:5
[task 2020-06-12T12:05:02.113Z] 12:05:02 INFO - #11 0x7fef37d01212 in XRE_XPCShellMain(int, char**, char**, XREShellData const*) /builds/worker/checkouts/gecko/js/xpconnect/src/XPCShellImpl.cpp:1204:10
[task 2020-06-12T12:05:02.113Z] 12:05:02 INFO - #12 0x558e09eebbcb in main /builds/worker/checkouts/gecko/js/xpconnect/shell/xpcshell.cpp:66:27
[task 2020-06-12T12:05:02.193Z] 12:05:02 INFO - #13 0x7fef30662b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
[task 2020-06-12T12:05:02.195Z] 12:05:02 INFO - SUMMARY: AddressSanitizer: heap-use-after-free /builds/worker/checkouts/gecko/js/src/builtin/String.cpp:3500:31 in SplitSingleCharHelper<unsigned char>
[task 2020-06-12T12:05:02.195Z] 12:05:02 INFO - Shadow bytes around the buggy address:
[task 2020-06-12T12:05:02.195Z] 12:05:02 INFO - 0x0c0c80007ca0: fd fd fd fd fd fd fd fd fa fa fa fa fd fd fd fd
[task 2020-06-12T12:05:02.195Z] 12:05:02 INFO - 0x0c0c80007cb0: fd fd fd fd fa fa fa fa fd fd fd fd fd fd fd fd
[task 2020-06-12T12:05:02.196Z] 12:05:02 INFO - 0x0c0c80007cc0: fa fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa
[task 2020-06-12T12:05:02.196Z] 12:05:02 INFO - 0x0c0c80007cd0: fd fd fd fd fd fd fd fd fa fa fa fa 00 00 00 00
[task 2020-06-12T12:05:02.197Z] 12:05:02 INFO - 0x0c0c80007ce0: 00 00 00 fa fa fa fa fa fd fd fd fd fd fd fd fa
[task 2020-06-12T12:05:02.197Z] 12:05:02 INFO - =>0x0c0c80007cf0: fa fa fa fa fd fd[fd]fd fd fd fd fa fa fa fa fa
[task 2020-06-12T12:05:02.198Z] 12:05:02 INFO - 0x0c0c80007d00: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
[task 2020-06-12T12:05:02.198Z] 12:05:02 INFO - 0x0c0c80007d10: 00 00 00 00 fa fa fa fa 00 00 00 00 00 00 00 00
[task 2020-06-12T12:05:02.198Z] 12:05:02 INFO - 0x0c0c80007d20: fa fa fa fa 00 00 00 00 00 00 00 00 fa fa fa fa
[task 2020-06-12T12:05:02.198Z] 12:05:02 INFO - 0x0c0c80007d30: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
[task 2020-06-12T12:05:02.199Z] 12:05:02 INFO - 0x0c0c80007d40: 00 00 00 00 fa fa fa fa 00 00 00 00 00 00 00 00
[task 2020-06-12T12:05:02.199Z] 12:05:02 INFO - Shadow byte legend (one shadow byte represents 8 application bytes):
[task 2020-06-12T12:05:02.199Z] 12:05:02 INFO - Addressable: 00
[task 2020-06-12T12:05:02.199Z] 12:05:02 INFO - Partially addressable: 01 02 03 04 05 06 07
[task 2020-06-12T12:05:02.199Z] 12:05:02 INFO - Heap left redzone: fa
[task 2020-06-12T12:05:02.199Z] 12:05:02 INFO - Freed heap region: fd
[task 2020-06-12T12:05:02.199Z] 12:05:02 INFO - Stack left redzone: f1
[task 2020-06-12T12:05:02.199Z] 12:05:02 INFO - Stack mid redzone: f2
[task 2020-06-12T12:05:02.199Z] 12:05:02 INFO - Stack right redzone: f3
[task 2020-06-12T12:05:02.199Z] 12:05:02 INFO - Stack after return: f5
[task 2020-06-12T12:05:02.199Z] 12:05:02 INFO - Stack use after scope: f8
[task 2020-06-12T12:05:02.199Z] 12:05:02 INFO - Global redzone: f9
[task 2020-06-12T12:05:02.199Z] 12:05:02 INFO - Global init order: f6
[task 2020-06-12T12:05:02.199Z] 12:05:02 INFO - Poisoned by user: f7
[task 2020-06-12T12:05:02.200Z] 12:05:02 INFO - Container overflow: fc
[task 2020-06-12T12:05:02.200Z] 12:05:02 INFO - Array cookie: ac
[task 2020-06-12T12:05:02.200Z] 12:05:02 INFO - Intra object redzone: bb
[task 2020-06-12T12:05:02.201Z] 12:05:02 INFO - ASan internal: fe
[task 2020-06-12T12:05:02.202Z] 12:05:02 INFO - Left alloca redzone: ca
[task 2020-06-12T12:05:02.202Z] 12:05:02 INFO - Right alloca redzone: cb
[task 2020-06-12T12:05:02.203Z] 12:05:02 INFO - Shadow gap: cc
[task 2020-06-12T12:05:02.203Z] 12:05:02 INFO - ==4720==ABORTING

Group: core-security → javascript-core-security

It looks like the string was freed by the GC off the main thread, so I'm going to assume it is some kind of GC rooting issue.

Component: JavaScript: Standard Library → JavaScript: GC
Keywords: csectype-uaf, sec-high
Assignee: nobody → sphink

(In reply to Andrew McCreight [:mccr8] from comment #1)

It looks like the string was freed by the GC off the main thread, so I'm going to assume it is some kind of GC rooting issue.

Most likely this is fallout from bug 1568923 (string deduplication), which would make it a different sort of rooting issue.

A nursery string was probably found to be identical to another string, so it discarded its own chars and then replaced itself with the now-tenured donor string. By itself, that couldn't cause this issue, because nothing points to the original chars that are being freed here. (When it discards its own chars, it drops them on the floor, but all nursery strings' chars allocations are maintained in a list of pointers that end up freed on a background thread, in the function freeUntracked that you see in the above stack.)

Where I've seen this sort of thing happen is when there is a dependent string that uses the chars from the deduplicated-away string. The deduplication machinery is supposed to update the dependent string's chars to the new chars pointer, or detect if that is not possible (eg the dependent string is in the tenured heap already) and mark the original nursery owner string as non-deduplicatable. If something went wrong in that machinery, the symptom would be what is observed here. (The non-deduplicatable marking could be considered a form of "rooting" the malloced chars. Kinda. Sorta.)

Severity: -- → S4
Priority: -- → P1
Has Regression Range: --- → yes
Keywords: regression

I will add a pref gate for this. I also intend to write some smaller, more targeted tests in hopes that fuzzing can find something here.

Status: NEW → ASSIGNED

After discussing this in our GC meeting, I realized that this is almost certainly due to something wrong with the handling of the non-deduplicatable bit. Especially since the code at the crash site is using AutoStableStringChars, which does the bit setting and clearing.

But there's an easy way to make this much, much safer and more conservative -- instead of setting the bit within the AutoStableStringChars and un-setting it (with logic for allowing nested AutoStableStringChars scopes), this now just sets it permanently. That does mean that any nursery string that has ever been used with AutoStableStringChars will be permanently banned from deduplication. Which honestly probably isn't that huge a deal; as soon as it's tenured, it's already ineligible.

Attachment #9162290 - Attachment description: Bug 1645415 - Be more conservative with deduplication → Bug 1645415 - Mark all relevant strings non-deduplicatable

Comment on attachment 9162290 [details]
Bug 1645415 - Mark all relevant strings non-deduplicatable

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: It would be relatively easy to trigger the flaw, since it just requires making multiple strings with the same content then doing some simple operations on them. That gets you read-only UAF on a string. So it's an information leak. I don't think it's exploitable.
  • Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
  • Which older supported branches are affected by this flaw?: beta
  • If not all supported branches, which bug introduced the flaw?: Bug 1568923
  • Do you have backports for the affected branches?: No
  • If not, how different, hard to create, and risky will they be?: This patch should backport cleanly.
  • How likely is this patch to cause regressions; how much testing does it need?: It's pretty safe. It sets more "do the safe thing" bits, and leaves them set for longer. The flaw it exposes is pretty hard to trigger, though, so it's tough to test. I will be working on that.
Attachment #9162290 - Flags: sec-approval?

Comment on attachment 9162290 [details]
Bug 1645415 - Mark all relevant strings non-deduplicatable

Approved to land and uplift (after conferring with relman)

Attachment #9162290 - Flags: sec-approval?
Attachment #9162290 - Flags: sec-approval+
Attachment #9162290 - Flags: approval-mozilla-beta+

https://hg.mozilla.org/mozilla-central/rev/221bb867f156

Group: javascript-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox80: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla80

(In reply to Steve Fink [:sfink] [:s:] from comment #6)

It would be relatively easy to trigger the flaw, since it just requires making multiple strings with the same content then doing some simple operations on them.

(Also in reply to Steve Fink [:sfink] [:s:] from comment #6)

The flaw it exposes is pretty hard to trigger, though, so it's tough to test. I will be working on that.

I'm being rather inconsistent here, aren't I? :-) I guess I'm talking about motivated attackers vs idiot me? (I haven't managed to be "doing some simple operations on [the right strings]" yet.)

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Flags: needinfo?(sphink)
Whiteboard: [sec-survey]
Flags: qe-verify-
Whiteboard: [sec-survey] → [sec-survey][post-critsmash-triage]
Flags: needinfo?(sphink)
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: