WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
16127
Reproducible crash inside PCRE under guard malloc
https://bugs.webkit.org/show_bug.cgi?id=16127
Summary
Reproducible crash inside PCRE under guard malloc
Mark Rowe (bdash)
Reported
2007-11-24 22:24:00 PST
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
Attachments
Reduced test case
(11 bytes, text/plain)
2007-11-24 22:24 PST
,
Mark Rowe (bdash)
no flags
Details
Crash log
(7.59 KB, text/plain)
2007-11-24 22:25 PST
,
Mark Rowe (bdash)
no flags
Details
Check against patternEnd to make sure we don't walk of the end of the string
(8.63 KB, patch)
2007-11-30 05:39 PST
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
Check against patternEnd to make sure we don't walk off the end of the string
(8.85 KB, patch)
2007-11-30 05:55 PST
,
Eric Seidel (no email)
darin
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Mark Rowe (bdash)
Comment 1
2007-11-24 22:24:40 PST
Created
attachment 17499
[details]
Reduced test case
Mark Rowe (bdash)
Comment 2
2007-11-24 22:25:23 PST
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)
Mark Rowe (bdash)
Comment 3
2007-11-24 22:26:28 PST
<
rdar://problem/5611792
>
Alexey Proskuryakov
Comment 4
2007-11-25 00:03:00 PST
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
***
Eric Seidel (no email)
Comment 5
2007-11-30 05:18:00 PST
bug 15839
no longer crashes on TOT, however this still does. Reopening.
Eric Seidel (no email)
Comment 6
2007-11-30 05:39:01 PST
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(-)
Eric Seidel (no email)
Comment 7
2007-11-30 05:55:42 PST
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(-)
Darin Adler
Comment 8
2007-11-30 09:30:02 PST
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.
Eric Seidel (no email)
Comment 9
2007-11-30 15:44:19 PST
r28260
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug