Loading a JavaScript snippet containing the following regexp literal will cause a crash under guard malloc: /\)[;\s]+/ This leads to indexing off the end of an array. My reading of the PCRE 7.4 sources suggest that they also have this problem. I recall seeing valgrind warnings in similar places when playing with it on Linux recently. Steps to reproduce: 1) Save attachment as test.js. 2) DYLD_INSERT_LIBRARIES=/usr/lib/libgmalloc.dylib DYLD_FRAMEWORK_PATH=WebKitBuild/Debug ./WebitBuild/Debug/testkjs test.js
Created attachment 17499 [details] Reduced test case
Created attachment 17500 [details] Crash log 0 com.apple.JavaScriptCore 0x0031a0f2 calculateCompiledPatternLengthAndFlags(unsigned short const*, int, JSRegExpIgnoreCaseOption, compile_data&, ErrorCode) + 1268 (pcre_compile.cpp:2550) 1 com.apple.JavaScriptCore 0x0031ab56 jsRegExpCompile(unsigned short const*, int, JSRegExpIgnoreCaseOption, JSRegExpMultilineOption, unsigned int*, char const**) + 82 (pcre_compile.cpp:3013) 2 com.apple.JavaScriptCore 0x002883df KJS::RegExp::RegExp(KJS::UString const&, KJS::UString const&) + 417 (regexp.cpp:70)
<rdar://problem/5611792>
I don't think that original PCRE has this problem - it relies on strings being null-terminated. *** This bug has been marked as a duplicate of 15839 ***
bug 15839 no longer crashes on TOT, however this still does. Reopening.
Created attachment 17603 [details] Check against patternEnd to make sure we don't walk of the end of the string JavaScriptCore/ChangeLog | 12 +++++ JavaScriptCore/pcre/pcre_compile.cpp | 47 ++++++++++---------- LayoutTests/ChangeLog | 10 ++++ .../fast/js/regexp-compile-crash-expected.txt | 5 ++ LayoutTests/fast/js/regexp-compile-crash.html | 22 +++++++++ 5 files changed, 72 insertions(+), 24 deletions(-)
Created attachment 17604 [details] Check against patternEnd to make sure we don't walk off the end of the string JavaScriptCore/ChangeLog | 12 +++++ JavaScriptCore/pcre/pcre_compile.cpp | 47 ++++++++++---------- LayoutTests/ChangeLog | 10 ++++ .../fast/js/regexp-compile-crash-expected.txt | 11 +++++ LayoutTests/fast/js/regexp-compile-crash.html | 22 +++++++++ 5 files changed, 78 insertions(+), 24 deletions(-)
Comment on attachment 17604 [details] Check against patternEnd to make sure we don't walk off the end of the string This is not the correct way to write tests in fast/js -- the actual test should be in a resources file, and the wrapper generated by the make-js-test-wrappers script. With that structure we can possibly run them standalone in the future. + while ((++ptr < patternEnd) && ((c = *ptr) != ']')) { if (c > 127) c = getCharAndAdvanceIfSurrogate(ptr); This still has a "run off the end" problem, because getCharAndAdvanceIfSurrogate doesn't check the end. But that problem will go away if we merge my change to remove support for UTF-16 surrogate pairs, currently up for review. I'm a little bit opposed to this code using parentheses around the patternEnd conditions, when all the other similar code in the same file omits parens in cases like these. r=me, but better to improve the tests.
r28260