Bug 47324 – REGRESSION(r68204-r68242): Crash during execution of String.replace with specific regular expression

• Component: JavaScriptCore
• Status: RESOLVED FIXED

Created attachment 70032 [details]
Crash report

Loading WebKit r69221 and executing the following line of JavaScript in the console results in a crash:
'"'.replace(/([^\\])?(["'])/g, '$1\\$2')

The crash occurs when the script is executed on the console after inspecting the start page, or a blank page. When other pages are viewed, results differ; for instance, running the script after loading http://www.apple.com/ results in the following output:
"!\""

Loading http://www.google.com.au/ and running the same script results in:
"!"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz{|}~\""

Neither of the above results is correct; expected result from Safari Version 5.0.2 [6533.18.5] and WebKit nightlies up to and including r68204 is "\"".

The exact output (and whether the script crashes or just returns unexpected results) depends on the page loaded and the build of WebKit. However, all WebKit nightlies from r68242 onwards exhibit incorrect behaviour.

This looks like it's related to the patch for https://bugs.webkit.org/show_bug.cgi?id=46404 (http://trac.webkit.org/changeset/68207)

[Alexey Proskuryakov]

I've got a crash in a debug build:

#0	0x101cb9300 in WTF::VectorBufferBase<unsigned short>::allocateBuffer at Vector.h:286
#1	0x101b652e5 in WTF::Vector<unsigned short, 0ul>::reserveCapacity at Vector.h:871
#2	0x101cba9fa in WTF::Vector<unsigned short, 0ul>::expandCapacity at Vector.h:788
#3	0x101b65452 in WTF::Vector<unsigned short, 0ul>::expandCapacity at Vector.h:795
#4	0x101cbaa4c in WTF::Vector<unsigned short, 0ul>::append<unsigned short> at Vector.h:931
#5	0x101ce116b in JSC::substituteBackreferencesSlow at StringPrototype.cpp:209
#6	0x101ce127e in JSC::substituteBackreferences at StringPrototype.cpp:223
#7	0x101ce1a55 in JSC::stringProtoFuncReplace at StringPrototype.cpp:402

That's because newCapacity was 18446744072277895851 (0xffffffffaaaaaaab AKA -1431655765). An obvious question: why didn't this crash nightlies? Is CRASH macro broken, or does newCapacity just happen to be different?

[Geoffrey Garen]

<rdar://problem/8526497>

[Michael Saboff]

Created attachment 70271 [details]
Patch adding check for valid subexpression index

This patch adds a check that the beginning index of a subexpression is not negative before using it in a replacement.

Also added the submitters test case to JS tests.

[WebKit Commit Bot]

Comment on attachment 70271 [details]
Patch adding check for valid subexpression index

Clearing flags on attachment: 70271

Committed r69422: <http://trac.webkit.org/changeset/69422>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.