1745564 - AddressSanitizer: heap-use-after-free [@ new_<const char &>] with READ of size 1 through [@ mozilla::intl::DisplayNames::ComputeDateTimeDisplayNames]

Status: RESOLVED FIXED

(Core :: JavaScript: Internationalization API, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20211210-026fe822049a (asan-opt build, run with --fuzzing-safe --ion-offthread-compile=off):

function b(c) {
  try {
    c()
  } catch  {}
}
d = b;
f = {};
addIntlExtras(f);
Intl.DisplayNames = f.DisplayNames;
dn = new Intl.DisplayNames("en", {
    type: "month"
})
d(() => dn.of())
dn.of(2)

Backtrace:

==21569==ERROR: AddressSanitizer: heap-use-after-free on address 0x60200000f070 at pc 0x5601ff97ea45 bp 0x7ffe0e78dad0 sp 0x7ffe0e78dac8
READ of size 1 at 0x60200000f070 thread T0
    #0 0x5601ff97ea44 in new_<const char &> dist/include/mozilla/Vector.h:155:12
    #1 0x5601ff97ea44 in copyConstruct<char> dist/include/mozilla/Vector.h:188:7
    #2 0x5601ff97ea44 in internalAppend<char> dist/include/mozilla/Vector.h:1396:3
    #3 0x5601ff97ea44 in append<char> dist/include/mozilla/Vector.h:1386:3
    #4 0x5601ff97ea44 in append<char> dist/include/mozilla/Vector.h:1469:10
    #5 0x5601ff97ea44 in mozilla::intl::DisplayNames::ComputeDateTimeDisplayNames(UDateFormatSymbolType, mozilla::Span<int const, 18446744073709551615ul>) intl/components/src/DisplayNames.cpp:144:20
    #6 0x5601ff8b3fb3 in mozilla::Result<mozilla::Ok, mozilla::intl::DisplayNamesError> mozilla::intl::DisplayNames::GetMonth<js::intl::FormatBuffer<char16_t, 32ul, js::TempAllocPolicy> >(js::intl::FormatBuffer<char16_t, 32ul, js::TempAllocPolicy>&, mozilla::intl::Month, mozilla::intl::DisplayNames::Fallback) dist/include/mozilla/intl/DisplayNames.h:784:13
    #7 0x5601ff8aff1f in js::intl_ComputeDisplayName(JSContext*, unsigned int, JS::Value*) js/src/builtin/intl/DisplayNames.cpp:461:13
    #8 0x5601ff12c824 in CallJSNative js/src/vm/Interpreter.cpp:388:13
    [...]
    #22 0x5601fece1cfb in _start (js+0x668cfb)

0x60200000f070 is located 0 bytes inside of 8-byte region [0x60200000f070,0x60200000f078)
freed by thread T0 here:
    #0 0x5601fed5e052 in free llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:111:3
    #1 0x5601ff8b060c in js_free dist/include/js/Utility.h:411:3
    #2 0x5601ff8b060c in operator() dist/include/js/Utility.h:629:38
    #3 0x5601ff8b060c in reset dist/include/mozilla/UniquePtr.h:424:7
    #4 0x5601ff8b060c in ~UniquePtr dist/include/mozilla/UniquePtr.h:385:18
    #5 0x5601ff8b060c in js::intl_ComputeDisplayName(JSContext*, unsigned int, JS::Value*) js/src/builtin/intl/DisplayNames.cpp:567:1
    #6 0x5601ff12c824 in CallJSNative js/src/vm/Interpreter.cpp:388:13
    [...]
    #19 0x7efd0b38d82f in __libc_start_main libc-start.c:291

Marking s-s due to use-after-free.

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Unable to reproduce bug 1745564 using build mozilla-central 20211210053159-026fe822049a. Without a baseline, bugmon is unable to analyze this bug.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 4

[Andrew McCreight [:mccr8]]

It looks like the initial allocation is on this line in intl_ComputeDisplayName: calendarChars = JS_EncodeStringToUTF8(cx, calendar);

Then it gets destroyed when we exit the function at the end, but apparently a pointer has escaped.

It looks like this code changed recently in bug 1719735. I'll mark it sec-high for now.

Greg, could you take a look? Thanks.

Comment 5

[Andrew McCreight [:mccr8]]

[Tracking Requested - why for this release]: use-after-free regression

Comment 6

[Greg Tatum [:gregtatum]]

I'm looking into it.

Comment 7

[Greg Tatum [:gregtatum]]

Calling Intl.DisplayNames.prototype.of twice will attempt to use the calendar UniqueChars from the previous call. Switching the calendar in options to a UniquePtr to own the data would fix it, as well as passing in the calendar each time. I'm going to opt for the latter.

Comment 8

[Greg Tatum [:gregtatum]]

Note that this issue is for MozExtension code, which means it's not exposed to the web, but may be called from browser chrome code.

Comment 9

[Greg Tatum [:gregtatum]]

Attachment: Bug 1745564 - Change the location of the calendar Span in DisplayNames; r?dminor

Comment 10

[Greg Tatum [:gregtatum]]

Comment on attachment 9255064 [details]
Bug 1745564 - Change the location of the calendar Span in DisplayNames; r?dminor

Security Approval Request

• How easily could an exploit be constructed based on the patch?: The JS API where this is triggered is not web-exposed, and is only exposed to browser chrome. I could not see anywhere where the JS APIs were exposed to custom user input. So it seems low likelihood. I don't have experience with read after frees and how to turn them into exploits, so I'm unsure how easy it is to turn them into exploits in general, but it doesn't seem like there is a big surface here to throw arbitrary data at it at least.
• 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?: Nightly-only
• If not all supported branches, which bug introduced the flaw?: Bug 1719735
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?:
• How likely is this patch to cause regressions; how much testing does it need?: Low. This is moving around how a string span is passed into the code. I'm fairly confident if it compiles, it won't cause regression. The intl/components gtests and SpiderMonkey test262 cover this code. I ran the gtests and test262 tests locally.

Comment 11

[Andrew McCreight [:mccr8]]

Comment on attachment 9255064 [details]
Bug 1745564 - Change the location of the calendar Span in DisplayNames; r?dminor

Nightly only issues don't need sec approval.

Comment 12

[Andrew McCreight [:mccr8]]

This can probably also be sec-moderate if it is only callable from privileged code.

Comment 13

[Greg Tatum [:gregtatum]]

Let me know if there is anything else I need to do. This is my first security bug, so I'm a bit unfamiliar with the process.

Comment 14

[Andrew McCreight [:mccr8]]

Nope, there's nothing more to do. You can go ahead and land it whenever.

Comment 15

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 1719735

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Change the location of the calendar Span in DisplayNames; r=dminor
https://hg.mozilla.org/integration/autoland/rev/23e753f4bab78da84e7be6698a3f7c5adfdbfb67
https://hg.mozilla.org/mozilla-central/rev/23e753f4bab7