1750935 - Differential Testing: Different output message involving RegExp and --fast-warmup

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

"x".split(RegExp())
"x".split(RegExp())
"x".split(RegExp())
"x".split(RegExp("[^]"))
"x".split(RegExp())
"x".split(RegExp())
"x".split(RegExp())
"x".split(RegExp())
print("\ue531\ue531\ue5310n".split(RegExp("\\d[^]")))

stdout: (also happens when you pass it in as a testcase)

$ ./js-dbg-64-linux-x86_64-89aa2c8696b7 --fuzzing-safe --differential-testing --no-threads
js> "x".split(RegExp())
["x"]
js> "x".split(RegExp())
["x"]
js> "x".split(RegExp())
["x"]
js> "x".split(RegExp("[^]"))
["", ""]
js> "x".split(RegExp())
["x"]
js> "x".split(RegExp())
["x"]
js> "x".split(RegExp())
["x"]
js> "x".split(RegExp())
["x"]
js> print("\ue531\ue531\ue5310n".split(RegExp("\\d[^]")))
111,
js> 

$ ./js-dbg-64-linux-x86_64-89aa2c8696b7 --fuzzing-safe --differential-testing --no-threads --fast-warmup
js> "x".split(RegExp())
["x"]
js> "x".split(RegExp())
["x"]
js> "x".split(RegExp())
["x"]
js> "x".split(RegExp("[^]"))
["", ""]
js> "x".split(RegExp())
["x"]
js> "x".split(RegExp())
["x"]
js> "x".split(RegExp())
["x"]
js> "x".split(RegExp())
["x"]
js> print("\ue531\ue531\ue5310n".split(RegExp("\\d[^]")))
,
js> 

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/962e589d9931
user:        Tooru Fujisawa
date:        Tue Jan 11 17:40:22 2022 +0000
summary:     Bug 1745664 - Part 2: Add length-3 static string for 100-255 range. r=nbp

Compile with AR=ar sh ./configure --enable-debug --with-ccache --enable-gczeal --enable-debug-symbols --disable-bootstrap --disable-tests, tested on m-c rev 89aa2c8696b7.

Setting s-s to be safe as a start as this involves a JIT flag (--fast-warmup) that is present in fuzz-flags.txt.

Comment 1

[Tooru Fujisawa [:arai]]

Thanks!

The issue is that the helper function I've added receives char parameters,
but callsites are templatized for char/char16_t/Latin1Char, where
char16_t causes lossy cast.
(I hoped --enable-warnings-as-errors catches...)

https://searchfox.org/mozilla-central/rev/d4b9c457db637fde655592d9e2048939b7ab2854/js/src/vm/StaticStrings.h#166-171

  static bool fitsInLength3Static(char c1, char c2, char c3, int* i) {
...
    if ('1' <= c1 && c1 < '3' && '0' <= c2 && c2 <= '9' && '0' <= c3 &&
        c3 <= '9') {

https://searchfox.org/mozilla-central/rev/d4b9c457db637fde655592d9e2048939b7ab2854/js/src/frontend/ParserAtom.h#593-623

class WellKnownParserAtoms {
...
  template <typename CharsT>
  TaggedParserAtomIndex lookupTinyIndex(CharsT chars, size_t length) const {
...
        if (StaticStrings::fitsInLength3Static(chars[0], chars[1], chars[2],
                                               &i)) {

https://searchfox.org/mozilla-central/rev/d4b9c457db637fde655592d9e2048939b7ab2854/js/src/vm/StaticStrings.h#102-132

class StaticStrings {
...
  /* Return null if no static atom exists for the given (chars, length). */
  template <typename CharT>
  MOZ_ALWAYS_INLINE JSAtom* lookup(const CharT* chars, size_t length) {
...
        if (fitsInLength3Static(chars[0], chars[1], chars[2], &i)) {

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Bug 1750935 - Fix static string lookup for two-byte. r?nbp!

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: affected chars

length-3 two-byte string consists of attached range are possibly affected. and if the sequence of lower bytes matches '100'-'255', they're accidentally converted into ASCII '100'-'255' strings.
This happens both in parse-time and runtime.

In term of security, this bug alone doesn't cause {code injection, sandbox escape, disclosure of user data, memory corruption, etc},
but if a security feature is implemented by JavaScript code, it can work wrongly.

Comment 4

[Tooru Fujisawa [:arai]]

This is nightly-only recent regression, so I'll land the patch without security approval.

about the rating, I think this matches sec-low or at most sec-moderate.

Comment 5

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/autoland/rev/e3024ec7c2935a43ea76d6b7fcb6d3f3c11ffc56

Comment 6

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

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

Comment 8

[Daniel Veditz [:dveditz]]

We'll call this sec-moderate because of the possibility that somewhere in our front-end code someone could get past a JS-implemented security mechanism (speculative -- probably not). Dupe bug 1751062 was reported within a few hours of this one so the bounty is split.