1523440 - Crash [@ bool InflateUTF8ToUTF16<(OnUTF8Error)3, bool UTF8EqualsChars<char16_t>(JS::UTF8Chars, char16_t const*)::{lambda(char16_t)#1}, JS::UTF8Chars>]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Christian Holler (:decoder)]

The attached testcase crashes on mozilla-central revision 06e3993985b7 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --enable-tests --enable-fuzzing --disable-debug --enable-address-sanitizer --disable-jemalloc --enable-optimize=-O2, run with FUZZER=BinAST ./fuzz-tests test.binjs).

Backtrace:

==3787==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x564804473f9f bp 0x7ffcdd5cdad0 sp 0x7ffcdd5cda80 T0)
==3787==The signal is caused by a WRITE memory access.
==3787==Hint: address points to the zero page.
    #0 0x564804473f9e in bool InflateUTF8ToUTF16<(OnUTF8Error)3, bool UTF8EqualsChars<char16_t>(JS::UTF8Chars, char16_t const*)::{lambda(char16_t)#1}, JS::UTF8Chars>(JSContext*, JS::UTF8Chars, bool UTF8EqualsChars<char16_t>(JS::UTF8Chars, char16_t const*)::{lambda(char16_t)#1}) js/src/vm/CharacterEncoding.cpp:346:11
    #1 0x564804473f9e in bool UTF8EqualsChars<char16_t>(JS::UTF8Chars, char16_t const*) js/src/vm/CharacterEncoding.cpp:594
    #2 0x5648046f951f in js::AtomHasher::match(js::AtomStateEntry const&, js::AtomHasher::Lookup const&) js/src/vm/JSAtom.cpp:146:14
    #3 0x5648046f951f in mozilla::detail::HashTable<js::AtomStateEntry const, mozilla::HashSet<js::AtomStateEntry, js::AtomHasher, js::SystemAllocPolicy>::SetHashPolicy, js::SystemAllocPolicy>::match(js::AtomStateEntry const&, js::AtomHasher::Lookup const&) dist/include/mozilla/HashTable.h:1688
    #4 0x5648046f951f in mozilla::detail::EntrySlot<js::AtomStateEntry const> mozilla::detail::HashTable<js::AtomStateEntry const, mozilla::HashSet<js::AtomStateEntry, js::AtomHasher, js::SystemAllocPolicy>::SetHashPolicy, js::SystemAllocPolicy>::lookup<(mozilla::detail::HashTable<js::AtomStateEntry const, mozilla::HashSet<js::AtomStateEntry, js::AtomHasher, js::SystemAllocPolicy>::SetHashPolicy, js::SystemAllocPolicy>::LookupReason)1>(js::AtomHasher::Lookup const&, unsigned int) const dist/include/mozilla/HashTable.h:1718
    #5 0x5648046f951f in mozilla::detail::HashTable<js::AtomStateEntry const, mozilla::HashSet<js::AtomStateEntry, js::AtomHasher, js::SystemAllocPolicy>::SetHashPolicy, js::SystemAllocPolicy>::lookupForAdd(js::AtomHasher::Lookup const&) dist/include/mozilla/HashTable.h:2053
    #6 0x5648046f951f in mozilla::HashSet<js::AtomStateEntry, js::AtomHasher, js::SystemAllocPolicy>::lookupForAdd(js::AtomHasher::Lookup const&) dist/include/mozilla/HashTable.h:606
    #7 0x5648046f951f in JSAtom* AtomizeAndCopyCharsFromLookup<AtomizeUTF8OrWTF8CharsWrapper<JS::WTF8Chars> >(JSContext*, AtomizeUTF8OrWTF8CharsWrapper<JS::WTF8Chars> const*, unsigned long, js::AtomHasher::Lookup const&, js::PinningBehavior, mozilla::Maybe<unsigned int> const&) js/src/vm/JSAtom.cpp:645
    #8 0x5648046f951f in JSAtom* AtomizeUTF8OrWTF8Chars<JS::WTF8Chars>(JSContext*, char const*, unsigned long) js/src/vm/JSAtom.cpp:999
    #9 0x564804f737ca in js::frontend::BinTokenReaderMultipart::readHeader() js/src/frontend/BinTokenReaderMultipart.cpp:179:7
    #10 0x564804f58620 in js::frontend::BinASTParserPerTokenizer<js::frontend::BinTokenReaderMultipart>::parseAux(js::frontend::GlobalSharedContext*, unsigned char const*, unsigned long, js::frontend::BinASTSourceMetadata**) js/src/frontend/BinASTParserPerTokenizer.cpp:117:3
    #11 0x564804f5971a in js::frontend::BinASTParserPerTokenizer<js::frontend::BinTokenReaderMultipart>::parse(js::frontend::GlobalSharedContext*, unsigned char const*, unsigned long, js::frontend::BinASTSourceMetadata**) js/src/frontend/BinASTParserPerTokenizer.cpp:94:17
    #12 0x564804f5971a in js::frontend::BinASTParserPerTokenizer<js::frontend::BinTokenReaderMultipart>::parse(js::frontend::GlobalSharedContext*, mozilla::Vector<unsigned char, 0ul, js::TempAllocPolicy> const&, js::frontend::BinASTSourceMetadata**) js/src/frontend/BinASTParserPerTokenizer.cpp:87
    #13 0x564804123c59 in testBinASTReaderFuzz(unsigned char const*, unsigned long) js/src/fuzz-tests/testBinASTReader.cpp:68:27
    #14 0x5648041c22cb in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) tools/fuzzing/libfuzzer/FuzzerLoop.cpp:517:13
    [...]
    #21 0x5648040273d8 in _start (/home/ubuntu/build/build/fuzz-tests+0x5723d8)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV js/src/vm/CharacterEncoding.cpp:346:11 in bool InflateUTF8ToUTF16<(OnUTF8Error)3, bool UTF8EqualsChars<char16_t>(JS::UTF8Chars, char16_t const*)::{lambda(char16_t)#1}, JS::UTF8Chars>(JSContext*, JS::UTF8Chars, bool UTF8EqualsChars<char16_t>(JS::UTF8Chars, char16_t const*)::{lambda(char16_t)#1})
==3787==ABORTING

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Tooru Fujisawa [:arai]]

uh, simply, we need to check that the input string is valid WTF-8.

Apparently not. this is an issue between UTF-8 vs WTF-8, that the internal supposes it's UTF-8 while API is for WTF-8

Comment 3

[Tooru Fujisawa [:arai]]

The patch won't have testcase, given we're not yet ready to create such testcase, that works across file format version bump.

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Handle WTF-8 in AtomHasher::Lookup.

AtomHasher::Lookup had to handle WTF-8 case separated from UTF-8 case.
(that's used from AtomizeUTF8OrWTF8Chars)
added the case and path.

Unfortunately I couldn't find a nice way to integrate the WTF-8 case handling in Lookup constructor, given the parameter type is same between UTF-8 and WTF-8,
so AtomizeUTF8OrWTF8Chars overrides type field after the ctor.

We could use UTF8Chars/WTF8Chars as parameter type, but the conversion from (char*,size_t)->UTF8Chars->(char*,size_t) around the ctor sounds bad.

Comment 5

[Jeff Walden [:Waldo]]

Comment on attachment 9039862 [details] [diff] [review]
Handle WTF-8 in AtomHasher::Lookup.

Review of attachment 9039862 [details] [diff] [review]:
-----------------------------------------------------------------

Oh if this doesn't just give us all a nice warm tingly feeling inside... 💩💩💩

::: js/src/vm/CharacterEncoding.cpp
@@ +564,5 @@
>  template bool GetUTF8AtomizationData<JS::WTF8Chars>(
>      JSContext* cx, const JS::WTF8Chars utf8, size_t* outlen,
>      JS::SmallestEncoding* encoding, HashNumber* hashNum);
>  
> +template <typename CharT, typename CharsT>

Make that |class CharsT| -- |JS::{U,W}TF8| are both classes.

Comment 6

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7bbee2f0125d3ac8e050c01c7ea150faad09ecd9
Bug 1523440 - Handle WTF-8 in AtomHasher::Lookup. r=jwalden

Comment 7

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/7bbee2f0125d