1906578 - UAF in [@ mozilla::detail::VectorImpl<T>::new_]

Status: RESOLVED DUPLICATE of bug 1890909

(Core :: JavaScript Engine, defect, P1)

Description

[Paul Bone [:pbone]]

Crash report: https://crash-stats.mozilla.org/report/index/fd8e3043-6779-4d7e-87cf-591ca0240518

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames:

0  xul.dll  mozilla::detail::VectorImpl<char16_t, 32, js::StringBufferAllocPolicy, 1>::ne...  mfbt/Vector.h:251
0  xul.dll  mozilla::detail::VectorImpl<char16_t, 32, js::StringBufferAllocPolicy, 1>::co...  mfbt/Vector.h:284
0  xul.dll  mozilla::Vector<char16_t, 32, js::StringBufferAllocPolicy>::internalAppend(ch...  mfbt/Vector.h:1443
0  xul.dll  mozilla::Vector<char16_t, 32, js::StringBufferAllocPolicy>::append(char16_t c...  mfbt/Vector.h:1433
0  xul.dll  mozilla::Vector<char16_t, 32, js::StringBufferAllocPolicy>::append(char16_t c...  mfbt/Vector.h:1516
0  xul.dll  js::StringBuffer::append(JSLinearString*)  js/src/util/StringBuffer.h:428
0  xul.dll  js::StringBuffer::append(JSString*)  js/src/util/StringBuffer.h:484
0  xul.dll  ArrayJoinDenseKernel(JSContext*, js::array_join::<lambda_0>, JS::Handle<js::N...  js/src/builtin/Array.cpp:1168
0  xul.dll  ArrayJoinKernel(JSContext*, js::array_join::<lambda_0>, JS::Handle<JSObject*>...  js/src/builtin/Array.cpp:1215
0  xul.dll  js::array_join(JSContext*, unsigned int, JS::Value*)  js/src/builtin/Array.cpp:1351

This is a PHC-annotated use-after-free that appeared at the end of April in Firefox 126. The memory error is for a 1024 byte object allocated at:

#0    JSRope::flatten(JSContext*) (xul.dll)
#1    ??? (firefox.exe)

And freed at:

#0    js::gc::BackgroundFreeTask::run(js::AutoLockHelperThreadState&) (xul.dll)
#1    js::GCParallelTask::runTask(JS::GCContext*, js::AutoLockHelperThreadState&) (xul.dll)
#2    js::GCParallelTask::runHelperThreadTask(js::AutoLockHelperThreadState&) (xul.dll)
#3    js::GlobalHelperThreadState::runOneTask(js::AutoLockHelperThreadState&) (xul.dll)
#4    mozilla::ThreadFuncPoolThread(void*) (xul.dll)
#5    _PR_NativeRunThread(void*) (nss3.dll)
#6    pr_root(void*) (nss3.dll)
#7    thread_start<unsigned int (__cdecl*)(void *),1> (ucrtbase.dll)
#8    BaseThreadInitThunk (KERNEL32.DLL)
#9    patched_BaseThreadInitThunk(int, void*, void*) (mozglue.dll)
#10    RtlUserThreadStart (ntdll.dll)

Comment 1

[Paul Bone [:pbone]]

I think this is the same cause:

https://crash-stats.mozilla.org/report/index/3de72e01-2d4c-4cae-8864-5cc720240427

Comment 2

[Paul Bone [:pbone]]

https://crash-stats.mozilla.org/report/index/facd0759-aca1-4768-999b-9f4240240518

Comment 3

[Paul Bone [:pbone]]

And another one with the same alloc/free stacks:

https://crash-stats.mozilla.org/report/index/f5ab88c5-5a20-4ae0-8160-386de0240518

Comment 4

[Gabriele Svelto [:gsvelto]]

It's hard to tell what's going on here, there's lots of crashes under these signatures that are detected as being potentially bit-flips, and several of the machines involved are very old which would make for a consistent pattern of bad hardware... however there's also some new machines mixed in, and more importantly many of these crashes are detected as potentially hitting a guard page. We know that bit-flip detection logic has false positives when dealing with buffer overflows (see rust-minidump issue #960) so the bit-flip frequency might be a false signal if what PHC is detecting are actual overflows.

Let's say that these signatures would be an excellent candidate for running memory tests on clients' machines to be able to figure out what's going on.

Comment 5

[Daniel Veditz [:dveditz]]

If you restrict the signatures to just the PHC-detected crashes they are all "25" for bit flip confidence -- none more or less. Seems unlikely; my confidence in the bit flip confidence is low :-)

Unless you changed something in the way PHC works in that release, this appears to be a regression in Firefox 126.
https://crash-stats.mozilla.org/search/?signature=%3Dmozilla%3A%3Adetail%3A%3AVectorImpl%3CT%3E%3A%3Anew_&signature=%3Dmemcpy%20%7C%20js%3A%3ACopyChars&signature=%3Dmozilla%3A%3AHashStringKnownLength&signature=%3Djs%3A%3Ajit%3A%3ACreateThisFromIC&phc_kind=%21__null__&date=%3E%3D2024-01-09T02%3A15%3A00.000Z&date=%3C2024-07-09T02%3A15%3A00.000Z&_facets=signature&_sort=date&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=address&_columns=reason&_columns=possible_bit_flips_max_confidence&_columns=phc_kind#crash-reports

Comment 6

[Daniel Veditz [:dveditz]]

There are zero PHC crashes in the Firefox 127 release. Is this a duplicate of something that got fixed? "WFM"?

There is one 127 nightly crash and one crash in 127.0b1 and nothing after that. The lack of beta crashes doesn't necessarily mean anything—there were only two crashes in 126 betas—but we don't see the explosion of release crashes we saw in 126.

Comment 7

[Daniel Veditz [:dveditz]]

There were about 50 PHC crashes a week until mid-June but only 12 in the last 3 weeks (since June 15). Firefox 127 was released on June 11.

According to the signature charts above, the general non-PHC crashes dropped from 50+ a day to about 20 around that same time.

Comment 8

[Gabriele Svelto [:gsvelto]]

(In reply to Daniel Veditz [:dveditz] from comment #5)

If you restrict the signatures to just the PHC-detected crashes they are all "25" for bit flip confidence -- none more or less. Seems unlikely; my confidence in the bit flip confidence is low :-)

The crashes are all detected as potential bit-flips because the addresses fall within a relatively large region which has been carved up into smaller pieces, with some holes in-between. Flipping a single bit puts us back in one of the memory areas between the holes which triggers the heuristic. I don't know if this is a side-effect of how PHC does allocations.

Comment 9

[Steve Fink [:sfink] [:s:]]

How can I symbolicate the PHC alloc stacks? I find it suspicious that the allocation is happening during string flattening. How many of them come from that?

If we're not seeing regular string allocation sites, that strongly implies to me a bug in the handling of the dependent strings created during flattening (as in, they are dependent on the characters allocated during flattening, those characters get freed, and then we try to read through the dependent string). We've had quite a few problems with that in the past, and a lot of recent code change in that area.

• Bug 1881995 landed in 127, and some fixes were backported to 127. Though one of them, bug 1894428, missed 127 and is only in 128.
• Bug 1879918 landed in 126, and an associated fix bug 1890909 was backported only to 127? And bug 1890909 would have exactly the stacks shown here.

Could this be bug 1890909? It's marked wontfix for 126, fixed in 127.

Comment 10

[Gabriele Svelto [:gsvelto]]

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

How can I symbolicate the PHC alloc stacks? I find it suspicious that the allocation is happening during string flattening. How many of them come from that?

You need phc-tools and a Socorro API token that allows you to access PII data and raw crash dumps. Once you have it run:

env SOCORRO_AUTH_TOKEN=<token> phc_symbolize.py --remote <crash_uuid>

Comment 11

[Steven DeTar [:sdetar]]

Making S2, P1 for now since is marked as a sec-high bug

Comment 12

[Steven DeTar [:sdetar]]

Steve, do you still believe this is a duplicate bug of bug 1890909? Should close this as a duplicate?

Comment 13

[Steve Fink [:sfink] [:s:]]

Yes, there is good evidence for it being a duplicate, and nobody has noticed anything that would argue that it isn't.