Bug 47324 - REGRESSION(r68204-r68242): Crash during execution of String.replace with specific regular expression
Summary: REGRESSION(r68204-r68242): Crash during execution of String.replace with spec...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.6
: P1 Major
Assignee: Michael Saboff
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-10-06 20:46 PDT by ben.dyer
Modified: 2010-10-08 14:32 PDT (History)
4 users (show)

See Also:


Attachments
Crash report (5.61 KB, text/plain)
2010-10-06 20:46 PDT, ben.dyer
no flags Details
Patch adding check for valid subexpression index (3.23 KB, patch)
2010-10-08 11:29 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ben.dyer 2010-10-06 20:46:29 PDT
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.
Comment 1 ben.dyer 2010-10-06 21:01:23 PDT
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)
Comment 2 Alexey Proskuryakov 2010-10-07 14:24:42 PDT
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?
Comment 3 Geoffrey Garen 2010-10-07 14:49:10 PDT
<rdar://problem/8526497>
Comment 4 Michael Saboff 2010-10-08 11:29:05 PDT
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.
Comment 5 WebKit Commit Bot 2010-10-08 14:32:54 PDT
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>
Comment 6 WebKit Commit Bot 2010-10-08 14:32:59 PDT
All reviewed patches have been landed.  Closing bug.