1742592 - Assertion failure: part.second > beginIndex, at js/src/builtin/intl/ListFormat.cpp:299

Status: VERIFIED FIXED

(Core :: JavaScript: Internationalization API, defect, P2)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20211123-71332992f78f (debug build, run with --fuzzing-safe --ion-offthread-compile=off):

function a(b, c) {
  b.formatToParts(c)
}
d = ["", "B"];
b = new Intl.ListFormat;
a(b, d);

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x00005555571fd772 in js::intl_FormatList(JSContext*, unsigned int, JS::Value*) ()
#1  0x0000555556c6ccb0 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) ()
[...]
#13 0x0000555556ac33f3 in main ()
rax	0x555555836725	93824995256101
rbx	0xfffb000000000000	-1407374883553280
rcx	0x5555581a5020	93825038700576
rdx	0x0	0
rsi	0x7ffff7105770	140737338431344
rdi	0x7ffff7104540	140737338426688
rbp	0x7fffffffbfa0	140737488338848
rsp	0x7fffffffbc20	140737488337952
r8	0x7ffff7105770	140737338431344
r9	0x7ffff7f99840	140737353717824
r10	0x0	0
r11	0x0	0
r12	0x7fffffffbcd0	140737488338128
r13	0x7fffffffbce8	140737488338152
r14	0x7fffffffbea0	140737488338592
r15	0x0	0
rip	0x5555571fd772 <js::intl_FormatList(JSContext*, unsigned int, JS::Value*)+6450>
=> 0x5555571fd772 <_ZN2js15intl_FormatListEP9JSContextjPN2JS5ValueE+6450>:	movl   $0x12b,0x0
   0x5555571fd77d <_ZN2js15intl_FormatListEP9JSContextjPN2JS5ValueE+6461>:	callq  0x555556b5a76b <abort>

Looking at the assert and surrounding code, this could probably cause an out-of-bounds access, so marking s-s.

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as reproducible on mozilla-central 20211123094249-71332992f78f.
The bug appears to have been introduced in the following build range:

Start: 51a532bcb9d05e50ca25fdcf6747b887d3f45c09 (20211101182421)
End: 08eb1047d841ce4f1fcee1595dc4ec7ed8cba8f5 (20211101215926)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=51a532bcb9d05e50ca25fdcf6747b887d3f45c09&tochange=08eb1047d841ce4f1fcee1595dc4ec7ed8cba8f5

Comment 4

[Nicolas B. Pierron [:nbp]]

The assertion seems to guard the computation of indexes within a dependent string, which suggest that if an attacker has some control over these values, this might lead to a random read access later on.

Zibi, André, any idea who would be the person person to look at this issue?

Comment 5

[André Bargull [:anba]]

This is just a wrong assertion condition. It was copied from DateTimeFormat, where we don't expect empty parts. But Intl.ListFormat can have empty parts, so the assertion should be changed from MOZ_ASSERT(part.second > beginIndex) to MOZ_ASSERT(part.second >= beginIndex).

Comment 6

[André Bargull [:anba]]

There's something else wrong:

let list = ["", "B"];
let lf = new Intl.ListFormat("en");

print(JSON.stringify(lf.formatToParts(list)));
print(lf.format(list));

prints:

[{"type":"literal","value":" and "},{"type":"element","value":""},{"type":"element","value":"B"}]
 and B

But the " and " element in the array should be in the middle instead of the first element.

Comment 7

[André Bargull [:anba]]

Hmm, okay. The other issue seems to be an ICU bug, because it's also reproducible in JSC and V8.

Comment 8

[Andrew McCreight [:mccr8]]

It sounds like this is an invalid assertion, and thus not a security issue? Can I go ahead and unhide this then? Thanks.

Comment 9

[André Bargull [:anba]]

Yes, this is just a bogus assertion.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 11

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Hi, anba
Could you point out where the ECMA 402 defines handling empty string for ListFormat? Or maybe some Unicode spec?
The reason I am asking because for lf.formatToParts(["", "B"])
in our implementation and V8, they both return {" and ", "", "B" },
however, I am not sure if this is correct. ( Are the first two elements " and ", "" necessary? )
I check ECMA 402 spec and icu source code, but didn't get any useful result.

Thanks

Comment 12

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

I'll fix the bogus assert first and file another bug to track the empty string problem,

Comment 13

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Bug 1742592 - Fix bogus assert in ListFormat.

Comment 14

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to André Bargull [:anba] from comment #6)

There's something else wrong:

let list = ["", "B"];
let lf = new Intl.ListFormat("en");

print(JSON.stringify(lf.formatToParts(list)));
print(lf.format(list));

prints:

[{"type":"literal","value":" and "},{"type":"element","value":""},{"type":"element","value":"B"}]
 and B

But the " and " element in the array should be in the middle instead of the first element.

Filed a bug in ICU Jira
https://unicode-org.atlassian.net/browse/ICU-21871

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/efc79ed34979
Fix bogus assert in ListFormat. r=tcampbell

Comment 16

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

https://hg.mozilla.org/mozilla-central/rev/efc79ed34979

Comment 17

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as fixed on rev mozilla-central 20211214042524-51773d1ab7b5.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.