122076 - RegExp crash when following link

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Karl Johan Kleist]

Build ID: 2002 01 24 03. Windows 2000.

The first crash (TB2168531Y) occurred when I clicked the link "About us"
on < http://www.marzie.com/ >.

The other one (TB2168574Y) occurred when I had clicked on the same link,
and the clicked the button "Go back one page".

The bug is intermittent, it seems.

Comment 1

[Karl Johan Kleist]

Sorry, I forgot to set severity "critical"

Comment 2

[R.B.]

Also on Linux 2002011821, talkback TB2170303Y.

Comment 3

[Jens Fallesen]

Just CVSup'ed tree about an hour ago and rebuilt on FreeBSD. Same problem here.
Seems to be platform independent.

Comment 4

[Adam Hauner]

WFM 2002012503/WinNT4

Comment 5

[Aaron Kaluszka]

WFM 2002012608/WinXP

Comment 6

[Anti Veeranna]

WFM, 2002012606/Linux

Comment 7

[David Gerard]

WFM 2002012605-0.9.8/WinNT4.

Comment 8

[Jens Fallesen]

Today's CVS pull and build (20020128) for FreeBSD works fine.

Comment 9

[Asa Dotzler [:asa]]

This crashes me with 021108 mozilla win32 build on win2K. Will try to get a
stack trace when talkback comes up. 

Comment 10

[Asa Dotzler [:asa]]

->JavaScript Engine. 

ParseAtom()
ParseQuantAtom()
ParseItem()
ParseAltern()
ParseRegExp()
ParseAtom()
ParseQuantAtom()
ParseItem()
ParseAltern()
ParseRegExp()
ParseAtom()
ParseQuantAtom()
ParseItem()
ParseAltern()
ParseRegExp()
ParseAtom()
ParseQuantAtom()
ParseItem()
ParseAltern()
ParseRegExp()
js_NewRegExp()
js_NewRegExpObject()
js_GetToken()
js_MatchToken()
ArgumentList()
MemberExpr()
UnaryExpr()
MulExpr()
AddExpr()
ShiftExpr()
RelExpr()
EqExpr()
BitAndExpr()
BitXorExpr()
BitOrExpr()
AndExpr()
OrExpr()
CondExpr()
AssignExpr()
Expr()
PrimaryExpr()
MemberExpr()
UnaryExpr()
MulExpr()
AddExpr()
ShiftExpr()
RelExpr()
EqExpr()
BitAndExpr()
BitXorExpr()
BitOrExpr()
AndExpr()
OrExpr()
CondExpr()
AssignExpr()
Expr()
Statement()
Statements()
FunctionBody()
FunctionDef()
FunctionStmt()
Statement()
Statements()
js_CompileTokenStream()
CompileTokenStream()
JS_CompileUCScriptForPrincipals()
JS_EvaluateUCScriptForPrincipals()
nsJSContext::EvaluateString()
nsScriptLoader::EvaluateScript()
nsScriptLoader::ProcessRequest()
nsScriptLoader::OnStreamComplete()
nsStreamLoader::OnStopRequest()
nsHttpChannel::OnStopRequest()
nsOnStopRequestEvent::HandleEvent()
nsARequestObserverEvent::HandlePLEvent()
PL_HandleEvent()
PL_ProcessPendingEvents()
nsEventQueueImpl::ProcessPendingEvents()
event_processor_callback()
our_gdk_io_invoke()
libglib-1.2.so.0 + 0xec10 (0x40367c10)
libglib-1.2.so.0 + 0x102d9 (0x403692d9)
libglib-1.2.so.0 + 0x108e3 (0x403698e3)
libglib-1.2.so.0 + 0x10a7c (0x40369a7c)
libgtk-1.2.so.0 + 0x8dd97 (0x4028cd97)
nsAppShell::Run()
nsAppShellService::Run()
main1()
main()
libc.so.6 + 0x18a42 (0x40466a42) 

Comment 11

[Phil Schwartau]

I got a crash when I loaded the site for the first time; that's it!
Same thing with my Linux build (both dated 2002-02-07). OS: Win ---> All.

Will attach WinNT stack trace below. It looks the same as Asa's. Since
this looks like it might be a RegExp crash, leaving assigned to rogerl.

Comment 12

[Phil Schwartau]

Attachment: WinNT stack trace

Comment 13

[Phil Schwartau]

Note: the crash does seem to be intermittent. One must be ready to debug
it the first time one loads the site; it may not crash again so easily -

Comment 14

[Phil Schwartau]

The regular expressions are in the external file

     view-source:http://www.marzie.com/include/scripts.js


Roger noted the problem seems to center on this one:

function checkURL ()
{
return 
(this.value.search(/^(((https?)|(ftp)):\/\/([\-\w]+\.)+\w{2,4}(\/[%\-\w]+(\.\w{2
,})?)*(([\w\-\.\?\\/\*\$+@&#;`~=%!]*)(\.\w{2,})?)*\/?)$/) != -1);
}

Comment 15

[Phil Schwartau]

Wow, I was able to load the site without crashing, and found this
error in the JS Console:

Error: unterminated parenthetical (
Source File: http://www.marzie.com/include/scripts.js
Line: 78, Column: 27
Source Code:
	return 
(this.value.search(/^(((https?)|(ftp)):\/\/([\-\w]+\.)+\w{2,4}(\/[%\-\w]+(\.\w{2
,})?)*(([\w\-\.\?\\/\*\$+@&#;`~=%!]*)(\.\w{2,})?)*\/?)$/) != -1);

Comment 16

[Phil Schwartau]

I have put the function checkURL() into a standalone JS shell testcase.
It crashes my JS shell, but intermittently. The test driver alone will
not be enough to verify the fix. We'll have to verify interactively,
by loading the testcase over and over by hand in the JS shell. And also,
of course, by loading the above website repeatedly.

In the meantime, I will attach the testcase to this bug before 
checking it into CVS; I'm not sure yet if this should be a "negative"
test or a "positive" test. That is, does checkURL() really contain
a syntax error, and JS should fail to compile it? Here is what NN4.7
says in its JS Console when we load the website:

          JavaScript Error: unterminated character class [


I think I see the error. What looks funny to me is this:


          / etc. etc.    [\w\-\.\?\\/    etc. etc. /   


I say, the \\/ is escaping the backslash so that it is a literal
and no longer functions as a backslash. Therefore the / which follows
is unescaped, and terminates the parse of the regular expression.
Since the character class beginning with [ has not closed yet, 
we get the error. Anyway, that's my theory. 


But why does IE6 load the site without an error?
See bug 101070, an Evangelism bug against Microsoft:

"microsoft.com - Using "/" character unescaped in RegExp literals is non-ECMA"

Comment 17

[Phil Schwartau]

Attachment: Standalone JS testcase; start JS shell and load() it

Comment 18

[Jord van der Elst]

I'll add to it. Just had another crash whilst visiting
http://www.statsman.org/genomestats/5.html ... I always find this page loading
slower in Mozilla than in IE, but sometimes I have it crashing totally in this
page (intermittent). No crash ID, as Mozilla won't quit on it's own.

It will just halt, and when CAD'ing out, I'm left with a desktop consisting of
white icons, and every icon I go over with the mouse goes white, system tray
will vanish, me needing to reboot.

Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:0.9.8+) Gecko/20020226
WinME

Comment 19

[Jord van der Elst]

missed something: am using Java Plug-in 1.3.1 for Netscape Navigator (DLL Helper)
NPOJI600.dll
NPJava11.dll
NPJava12.dll
NPJava131.dll
NPJava32.dll

Comment 20

[Phil Schwartau]

Jorden: thanks, but that looks like a completely different crash.
The one in this report is due to a RegExp crash in the JavaScript
Engine; yours looks like a Java problem.

Could you file a separate bug on that? (To the Plug-ins component).
Please cc me on it, too: [e-mail]. Thanks -

Comment 21

[Phil Schwartau]

Testcase added to JS testsuite:

         mozilla/js/tests/ecma_3/RegExp/regress-122076.js

However, it does not crash when I run it under the test driver.
See my Comment #16 above; the only way I will be able to verify
any fix for this bug is to start the JS shell and load() the 
testcase attached in Comment #17.

It's one of those peculiar crashes that requires just the right
combination of circumstances to occur...

Comment 22

[Phil Schwartau]

Adding "RegExp" to the summary to make this bug more easily searched -

Comment 23

[Phil Schwartau]

*** Bug 138326 has been marked as a duplicate of this bug. ***

Comment 24

[Phil Schwartau]

cc'ing Matti from bug 138326; noting we must verify that site
as well as this bug's site once the fix goes in -

Comment 25

[Chris Gonyea]

Is anyone working on fixing this bug? This bug is one of the few problems
remaining that prevents me from using Mozilla full time without switching back
to IE6.

Comment 26

[Phil Schwartau]

Our RegExp owner is away on sabbatical; that accounts for the delay.
Sorry for the inconvenience - he's back on June 1.

Comment 27

[Chris Gonyea]

Ok that's understandable. I can live with this until then. Thanks for replying.

Comment 28

[rogerl (gone)]

Attachment: Prevent over-reaching for end check

Nuts, I think I created this when fixing 98306. The '[' parsing loop is trying
to protect itself from running off the end of the regexp but is actually
looking too far ahead by comparing cp+1 against cpend - cp has already been
incremented at this point.
{Phil, I can't think of any better way of testing this, the trouble is that the
parse loop is going to keep marching through memory until a read fails because
it's an invalid address or it accidentally encounters a ']' - there's no
telling which will happen first. But note that running under Purify catches the
bad read}

Comment 29

[Phil Schwartau]

cc'ing reviewers for this patch -

Comment 30

[timeless]

Comment on attachment 85605 [details] [diff] [review]
Prevent over-reaching for end check

ok, i ran this test using valgrind on boffo and the current codebase results in

==12179== Invalid read of size 2
==12179==    at 0x402CA854: ParseAtom
(/mnt/ibm/mozhack/mozilla/js/src/jsregexp.c:916)
==12179==    by 0x402CA0C7: ParseQuantAtom
(/mnt/ibm/mozhack/mozilla/js/src/jsregexp.c:655)
...
==12179==    Address 0x4167ED88 is 0 bytes after a block of size 160 alloc'd
==12179==    at 0x40046D2B: malloc (vg_clientfuncs.c:100)
==12179==    by 0x40255237: JS_malloc
(/mnt/ibm/mozhack/mozilla/js/src/jsapi.c:1424)
==12179==    by 0x402E4597: js_NewStringCopyN
(/mnt/ibm/mozhack/mozilla/js/src/jsstr.c:2550)
==12179==    by 0x402D1CBE: js_NewRegExpObject
(/mnt/ibm/mozhack/mozilla/js/src/jsregexp.c:2835)

With this patch I don't get the invalid read and I do get the correct error.
If I were a peer i'd mark r+. After a bit of reading i even understand why this
is correct :)

Phil, you should probably be able to build a testdriver which integrates
valgrind, 
./run-mozilla.sh /usr/local/bin/valgrind --logfile-fd=3 ./xpcshell jstest 3>
valgrind.logfile
grep "Invalid read" valgrind.logfile

==12234==    definitely lost: 60 bytes in 3 blocks.
fwiw is bug 188206

Comment 31

[Brendan Eich [:brendan]]

Comment on attachment 85605 [details] [diff] [review]
Prevent over-reaching for end check

Please get this in for 1.4a.

/be

Comment 32

[rogerl (gone)]

checked in.

Comment 33

[Phil Schwartau]

Verified FIXED.

By using source from 2003-03-18, I was able to reproduce the crash in
the debug and optimized JS shell by loading the standalone JS testcase
from Comment #17 over and over. I was also able to reproduce the crash
in a 2003-03-18 Mozilla trunk build by loading the HTML testcase from
bug 138326 comment 10 over and over. Reproduced on WinNT and Linux.

Using today's source and today's Mozilla trunk build, I am unable to
reproduce the crash on WinNT or Linux with either testcase, no matter 
how many times I load them. 

Instead, I correctly get the same error as in Nav4 (see Comment #16):

             Error: unterminated character class [