13 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Crash in jsregexp.c → Very non-greedy regexp causes crash in jsregexp.c
In a 184.108.40.206-ish debug build I crash dereferencing a null "result->sz" in PushBackTrackState. Didn't notice a particularly large amount of memory use though, so maybe I'm hitting something different given some of the other js fixes that went into 220.127.116.11
Assignee: general → mrbkap
This looks like a non-exploitable null pointer dereference caused (in part) by a failure to propagate OOM. I want to look and see why we're spinning out of control here with the non-greedy regexps only, though.
Priority: -- → P1
Target Milestone: --- → mozilla1.9alpha
Status: NEW → ASSIGNED
This fixes the crash due to the OOM that we're hitting. We shouldn't be hitting an OOM condition at all, though. I'll file a new bug on fixing the non-greedy quantifiers to not be wrong.
Comment on attachment 215298 [details] [diff] [review] Fix the crash Actually, I have a more complete patch that fixes both problems.
As far as I can tell, this does the right thing.
Attachment #215337 - Flags: review?(brendan)
Comment on attachment 215337 [details] [diff] [review] More complete fix Good as far as it goes, but not the complete fix. /be
Attachment #215337 - Flags: review?(brendan) → review+
Based on comment 2, can we remove the security flag?
Whiteboard: [sg:nse] null deref
(In reply to comment #8) > Based on comment 2, can we remove the security flag? Yes, we can. Clearing the security-sensitive flag.
Comment on attachment 215337 [details] [diff] [review] More complete fix This actually causes worse crashes.
Attachment #215337 - Flags: review+ → review-
Checking in regress-330352.js; /cvsroot/mozilla/js/tests/js1_5/Regress/regress-330352.js,v <-- regress-330352.js initial revision: 1.1
mrbkap, are you going to be able to continue working on this patch?
options("relimit") doesn't help, fwiw. options("relimit"); /(a*?)+?/.exec(""); still causes js to consume all available memory.
Taking this one from mrbkap so I won't forget to work on it.
Assignee: mrbkap → crowder
Status: ASSIGNED → NEW
This is a similar bug to bug 367888: we have a match-case which doesn't properly consume an empty match, causing the "repeat" code to continue repeating forever. The usual pieces (ie., the parts that didn't fail before) of the test-suite pass with this patch applied.
Attachment #259385 - Flags: review?(mrbkap)
Here's the perl behavior on the same match, for reference: $ perl -e '"AB" =~ /(.*?)*?B/; print "$&, $1\n"' AB, A
Attachment #259385 - Flags: review?(mrbkap) → review+
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
verified fixed linux, windows, mac* shell 20070406
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.