500644 - (CVE-2009-3372) PAC: crash when using PAC-based manual proxy config and the attached testcase

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[marcoc120]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5 (.NET CLR 3.5.30729)

When opening/rendering the attached HTML page with Firefox 3.5rc2/rc3 on Windows XP SP3, the browser consistently crashes. The problem was first observed on Firefox 3.5b99.

The same issue was not observed using the same browser version but on Windows XP SP2 or on the MacOS X version of the same browser. The same page opens fine with Firefox 3.0.x on all platforms including Win XP SP3.

We did not test it on Windows Vista.

The same behavior was reproducible when running Firefox 3.5 in safe-mode.

Reproducible: Always

Steps to Reproduce:
1. Download the isolated test case zip file at the following URL: http://www.mediafire.com/file/rjqmznznbmo/ff3crash.zip
2. Unzip the test case in the "htdocs" directory of a web server. We used Apache 2.2
3. Access the web page using Firefox 3.5rc3 running on a Windows XP SP3. The URL will be: http://<your-server>/ff3crash/ff35-crash.html

Actual Results:  
The Firefox 3.5 browser crashes.


Expected Results:  
The browser should not crash - the rendered page is just a blank white page.


The test page imports JavaScript/CSS from YUI (Yahoo User Interface toolkit) and TinyMCE (the popular JavaScript-based RichTextEditor). This isolated test case is a simplified version of a HTML page from an Oracle product which is already in production. The same page has been working fine when accessed with Firefox 3.0.x on all platforms. It is crucial for Oracle to have this bug fixed before Firefox 3.5 goes production.

Comment 1

[Dietrich Ayala (:dietrich)]

I downloaded the testcase and cannot reproduce this on Mac: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090626 Shiretoko/3.5pre

Comment 2

[Matthias Versen [:Matti]]

Please read https://developer.mozilla.org/en/How_to_get_a_stacktrace_for_a_bug_report

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Reporter: does turning javascript.options.jit.content to false in about:config prevent the crash?

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

FWIW, I can't get this to crash using Windows 7, either. Any chance you're running an add-on that's causing problems? Make sure to disable things like Firebug, just in case.

Comment 5

[Dietrich Ayala (:dietrich)]

uploaded the testcase to http://people.mozilla.org/~dietrich/ff3crash/

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

And I can't reproduce in XP, either. We'll either need a different testcase, or details on how your server is configured, I think.

Please re-open if you can provide more details/help us understand this better.

As for blocking, I'm not sure that we'd be able to block the final release on this issue if it's Oracle only, though we'd definitely want to investigate a fix (if a problem exists) before we issue 3.5.1 and the Major Update offer.

Comment 7

[marcoc120]

Are you trying on XP with Service Pack 3 installed?

The problem is not reproducible on XP with Service Pack 2 nor in other platforms like Mac OS X.

Comment 8

[marcoc120]

As for disabling all the add-on and plugins, we could reproduce it even with Firefox running in safe-mode with everything disabled.

As mentioned in the steps above, you MUST access the test page through a web server to hit the issue. Opening the test case as a file will not present the problem.

Comment 9

[marcoc120]

Setting javascript.options.jit.content to false did not avoid the crash.
The page loaded the first time after the property change but Firefox crashed on the first reload of the page. After that, it consistently crashed again.

Comment 10

[marcoc120]

Here are of the stack traces for the crashing thread - matti, thx a lot for the link with the info:


ID: c29cb575-1093-46f8-84b7-3615a2090626
ID: e922cb97-76f3-4c65-ba10-b83dd2090626
Signature: nsCacheService::DoomEntry_Internal(nsCacheEntry*)
0  	xul.dll  	nsCacheService::DoomEntry_Internal(nsCacheEntry*)  	netwerk/cache/src/nsCacheService.cpp:1479
1 	xul.dll 	nsCacheEntryDescriptor::Doom() 	netwerk/cache/src/nsCacheEntryDescriptor.cpp:398
2 	xul.dll 	nsHttpChannel::CloseCacheEntry(int) 	netwerk/protocol/http/src/nsHttpChannel.cpp:2388
3 	xul.dll 	nsHttpChannel::OnStopRequest(nsIRequest*,nsISupports*,unsigned int) 	netwerk/protocol/http/src/nsHttpChannel.cpp:4990
4 	xul.dll 	nsInputStreamPump::OnStateStop() 	netwerk/base/src/nsInputStreamPump.cpp:576
5 	xul.dll 	nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) 	netwerk/base/src/nsInputStreamPump.cpp:401
6 	xul.dll 	nsInputStreamReadyEvent::Run() 	xpcom/io/nsStreamUtils.cpp:111
7 	xul.dll 	nsThread::ProcessNextEvent(int,int*) 	xpcom/threads/nsThread.cpp:510
8 	xul.dll 	nsBaseAppShell::Run() 	widget/src/xpwidgets/nsBaseAppShell.cpp:170
9 	xul.dll 	nsAppStartup::Run() 	toolkit/components/startup/src/nsAppStartup.cpp:193
10 	nspr4.dll 	PR_GetEnv 	
11 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:110
12 	firefox.exe 	firefox.exe@0x21a7 	
13 	kernel32.dll 	kernel32.dll@0x17076


ID: 0e00b519-19a7-470f-aecf-c48c02090626
ID: a6c8d6bb-4ee5-4a8a-9256-fbe5d2090625
0  	xul.dll  	xul.dll@0x309eac  	
1 	xul.dll 	nsCacheService::DoomEntry_Internal(nsCacheEntry*) 	netwerk/cache/src/nsCacheService.cpp:1493
2 	xul.dll 	nsCacheEntryDescriptor::Doom() 	netwerk/cache/src/nsCacheEntryDescriptor.cpp:398
3 	xul.dll 	nsHttpChannel::CloseCacheEntry(int) 	netwerk/protocol/http/src/nsHttpChannel.cpp:2388
4 	xul.dll 	nsHttpChannel::OnStopRequest(nsIRequest*,nsISupports*,unsigned int) 	netwerk/protocol/http/src/nsHttpChannel.cpp:4990
5 	xul.dll 	nsInputStreamPump::OnStateStop() 	netwerk/base/src/nsInputStreamPump.cpp:576
6 	xul.dll 	nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) 	netwerk/base/src/nsInputStreamPump.cpp:401
7 	xul.dll 	nsInputStreamReadyEvent::Run() 	xpcom/io/nsStreamUtils.cpp:111
8 	xul.dll 	nsThread::ProcessNextEvent(int,int*) 	xpcom/threads/nsThread.cpp:510
9 	xul.dll 	nsBaseAppShell::Run() 	widget/src/xpwidgets/nsBaseAppShell.cpp:170
10 	xul.dll 	nsAppStartup::Run() 	toolkit/components/startup/src/nsAppStartup.cpp:193
11 	nspr4.dll 	PR_GetEnv 	
12 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:110
13 	firefox.exe 	firefox.exe@0x21a7 	
14 	kernel32.dll 	kernel32.dll@0x17076



ID: e5249f21-8f35-46c7-864d-430662090626
ID: cfba9fb4-0ebd-4bde-89c9-e33992090626
ID: 9c9874fd-a2d4-441c-9362-39f392090625
ID: 1292aa27-5f99-40f2-9284-8ad702090625
0  	kernel32.dll  	kernel32.dll@0x12afb  	
1 	mozcrt19.dll 	_CxxThrowException 	throw.cpp:159
2 	mozcrt19.dll 	operator new(unsigned int) 	obj-firefox/memory/jemalloc/src/new.cpp:57
3 	xul.dll 	nsScriptErrorConstructor 	js/src/xpconnect/src/xpcmodule.cpp:69
4 	xul.dll 	nsGenericFactory::CreateInstance(nsISupports*,nsID const&,void**) 	obj-firefox/xpcom/build/nsGenericFactory.cpp:80
5 	xul.dll 	nsCreateInstanceByContractID::operator()(nsID const&,void**) 	obj-firefox/xpcom/build/nsComponentManagerUtils.cpp:210
6 	xul.dll 	nsCOMPtr_base::assign_from_gs_contractid(nsGetServiceByContractID,nsID const&) 	obj-firefox/xpcom/build/nsCOMPtr.cpp:134 



ID: a0cea92e-f6aa-4f88-b32e-1ce212090626
Signature: nsPurpleBuffer::SelectPointers(GCGraphBuilder&)
0  	xul.dll  	nsPurpleBuffer::SelectPointers(GCGraphBuilder&)  	xpcom/base/nsCycleCollector.cpp:860
1 	xul.dll 	xul.dll@0x9d0553 	
2 	xul.dll 	XPCCycleCollectGCCallback 	js/src/xpconnect/src/nsXPConnect.cpp:390
3 	js3250.dll 	js_GC 	js/src/jsgc.cpp:3500
4 	js3250.dll 	JS_GC 	js/src/jsapi.cpp:2458
5 	xul.dll 	nsXPConnect::Collect() 	js/src/xpconnect/src/nsXPConnect.cpp:477
6 	xul.dll 	nsCycleCollector::Collect(unsigned int) 	xpcom/base/nsCycleCollector.cpp:2340
7 	xul.dll 	nsCycleCollector_collect() 	xpcom/base/nsCycleCollector.cpp:2999
8 	xul.dll 	nsJSContext::CC() 	dom/src/base/nsJSEnvironment.cpp:3454
9 	xul.dll 	nsCCMemoryPressureObserver::Observe(nsISupports*,char const*,unsigned short const*) 	dom/src/base/nsJSEnvironment.cpp:307
10 	xul.dll 	nsObserverService::NotifyObservers(nsISupports*,char const*,unsigned short const*) 	xpcom/ds/nsObserverService.cpp:181
11 	xul.dll 	nsMemoryImpl::RunFlushers(unsigned short const*) 	xpcom/base/nsMemoryImpl.cpp:268
12 	xul.dll 	nsMemoryImpl::FlushEvent::Run() 	xpcom/base/nsMemoryImpl.cpp:283
13 	xul.dll 	nsThread::ProcessNextEvent(int,int*) 	xpcom/threads/nsThread.cpp:510
14 	xul.dll 	nsBaseAppShell::Run() 	widget/src/xpwidgets/nsBaseAppShell.cpp:170
15 	xul.dll 	nsAppStartup::Run() 	toolkit/components/startup/src/nsAppStartup.cpp:193
16 	nspr4.dll 	PR_GetEnv 	
17 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:110
18 	firefox.exe 	firefox.exe@0x21a7 	
19 	kernel32.dll 	kernel32.dll@0x17076


ID: 7e5baf99-a85c-4479-b4ec-198b82090625
Signature: nsCacheEntryHashTable::HashKey(PLDHashTable*, void const*)
0  	xul.dll  	nsCacheEntryHashTable::HashKey(PLDHashTable*,void const*)  	netwerk/cache/src/nsCacheEntry.cpp:506
1 	xul.dll 	PL_DHashTableOperate 	obj-firefox/xpcom/build/pldhash.c:599
2 	xul.dll 	nsMemoryCacheDevice::EvictionList(nsCacheEntry*,int) 	netwerk/cache/src/nsMemoryCacheDevice.cpp:404
3 	xul.dll 	nsCacheEntryHashTable::AddEntry(nsCacheEntry*) 	netwerk/cache/src/nsCacheEntry.cpp:462
4 	xul.dll 	nsMemoryCacheDevice::BindEntry(nsCacheEntry*) 	netwerk/cache/src/nsMemoryCacheDevice.cpp:200
5 	xul.dll 	nsCacheService::EnsureEntryHasDevice(nsCacheEntry*) 	netwerk/cache/src/nsCacheService.cpp:1431
6 	xul.dll 	nsCacheEntryDescriptor::nsOutputStreamWrapper::LazyInit() 	netwerk/cache/src/nsCacheEntryDescriptor.cpp:588
7 	xul.dll 	nsCacheEntryDescriptor::nsOutputStreamWrapper::`scalar deleting destructor'(unsigned int) 	
8 	xul.dll 	nsCacheEntryDescriptor::nsOutputStreamWrapper::Release() 	netwerk/cache/src/nsCacheEntryDescriptor.cpp:571
9 	xul.dll 	xul.dll@0x8a006f 	
10 	xul.dll 	nsInputStreamPump::OnStateStop() 	netwerk/base/src/nsInputStreamPump.cpp:576
11 	xul.dll 	nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) 	netwerk/base/src/nsInputStreamPump.cpp:401
12 	xul.dll 	nsInputStreamReadyEvent::Run() 	xpcom/io/nsStreamUtils.cpp:111
13 	xul.dll 	nsThread::ProcessNextEvent(int,int*) 	xpcom/threads/nsThread.cpp:510
14 	xul.dll 	nsBaseAppShell::Run() 	widget/src/xpwidgets/nsBaseAppShell.cpp:170
15 	xul.dll 	nsAppStartup::Run() 	toolkit/components/startup/src/nsAppStartup.cpp:193
16 	nspr4.dll 	PR_GetEnv 	
17 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:110
18 	firefox.exe 	firefox.exe@0x21a7 	
19 	kernel32.dll 	kernel32.dll@0x17076

Comment 11

[marcoc120]

More crash IDs pointing to the same stack trace:

ID: b2f514c2-85e8-40b3-b6ee-da1fb2090626
ID: ada79566-ed7c-410a-9c71-705082090626
ID: 94e25869-1e71-4340-b9dc-ee4fb2090626
Signature: nsCacheService::DoomEntry_Internal(nsCacheEntry*)

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

We've tried in Windows 7 RC, Windows Vista, and Windows XP SP3, going through a web server, and can't reproduce this at all.

I've asked in private email if I can get a URL to an external webserver where you're seeing your minimal testcase crashing. That would help a lot. Also, in the crash report cited via email:

http://crash-stats.mozilla.com/report/index/ba222606-7836-457e-bbf4-f7c092090626

we noticed GoogleDesktopMozilla.dll is in your modules list; we've had problems before where an old, out of date version of that application has caused problems. Can you go into your Control Panel and remove Google Desktop - you can then go back to the website and install a new version, but it's important you do the "Remove" from Control Panel to clean up the DLL.

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #11)
> Signature: nsCacheService::DoomEntry_Internal(nsCacheEntry*)

Yeah, there's something borking the network request, basically, and failing at cleanup AIUI. We've had problems with TinyMCE before, but nothing that's looked like this (see bug 494883) but those have been related to JIT, which comment 9 indicates isn't what's happening here.

Comment 14

[marcoc120]

I was not the one reporting crash ba222606-7836-457e-bbf4-f7c092090626 via mail - I do not see that crash ID in the about:crashes page.
In fact I do not have GoogleDesktop installed in my box.

I guess somebody else is seeing the same failure.

We were actually able to re-produce the issue on a clean XP SP3 image (IE6-XPSP3.EXE) downloaded at http://www.microsoft.com/Downloads/details.aspx?FamilyID=21eabb90-958f-4b64-b5f1-73d0a413c8ef&displaylang=en

We just downloaded the image and installed Firefox in it.
The crash ID for that configuration is: a4384702-fb89-4bef-9afc-69ab72090626

We are looking at hosting this outside on the internet. Stay tuned.

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

> I guess somebody else is seeing the same failure.

(It's one of your colleagues; the call is coming from inside the firewall!)

Comment 16

[marcoc120]

Provided access to a hosted instance of the test page to Mike over email.

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

We've been able to confirm that this crash only happens when a user is connected via the internal Oracle network, or when logged in using the Cisco VPN client to Oracle's internal network.

This is odd.

Further, we've narrowed the regression range to:

No crash:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090522 Shiretoko/3.5pre
 
Crash:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090523 Shiretoko/3.5pre
http://crash-stats.mozilla.com/report/index/39482924-6f7d-4a3e-ae90-e7ace2090626

(even odder: that crash stack doesn't match the other ones listed here; perhaps that's a fluke?)

The changes in the regression range:

http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?startdate=May+22+2009&enddate=May+23+2009

don't seem to touch the networking stack.

Comment 18

[Mike Beltzner [:beltzner, not reading bugmail]]

We were able to confirm this crash is related to the PAC.

Possibly related bugs are https://bugzilla.mozilla.org/buglist.cgi?quicksearch=%22pac%22%20component:networking

The Oracle guys are trying to get permission to attach their PAC file in order to help us figure out what's causing the crash.

Comment 19

[timeless]

so, https://developer.mozilla.org/en/How_to_get_a_stacktrace_with_WinDbg would enable them to setup symbols (and we can easily get sources too), if someone can visit irc during hours i'm around, we can arrange to investigate local variables for at least the cache portion of the crash.

note that crashing in crt could be OOM, the _CxxThrowException family is OOM.
nsPurpleBuffer::SelectPointers under GC /could/ be related to OOM (I haven't looked, but know that some of my code in that area is bad, i need to spend some time w/ a CC person on it).

Comment 22

[mgueury]

I got that same issue (working at the same company that all people above)

Disabling the automatic proxy URL solves the issue.
- with or without extension
- with a new or a old profile.

From my experience, I think it happens since 3.5b99. It was working before.

Comment 24

[mgueury]

I see 2 ways forwards:
1) ORACLE NIS team changes his PAC file to use smaller regular expressions
2) Mozilla team fixes the issue about too long regular 
   expression in PAC file causing crashes in a lot of place in Mozilla 
   I crashed for me always with different stacks and often memory related issues.

Thanks

Comment 25

[Samuel Sidler (old account; do not CC)]

Who works on PAC code? ...

Comment 26

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

If it's a regular expression bug, can we not get a test case reduced that just calls that large regexp in script?  In that case, it should go over to the JS Engine component for dmandelin to sweat over.

(There might also be relevant fixes already in 3.5.1 or coming in 3.5.2 -- someone should test that, too!)

Comment 27

[mgueury]

The problem is the same in 3.5.1. (Not tested 3.5.2 pre)

Comment 28

[Peter Ondrejka]

Please note this issue affects a company with the size of 80 000 employees. Every user using Firefox is affected.

Thanks,
Peter

Comment 29

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Yeah, my gut says this is regexp-related, probably regexp-JIT-related.  Moving to JS engine, adding dmandelin.  Dave: see comment 23.

Comment 30

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Sigh, really adding dmandelin.

Comment 31

[David Mandelin [:dmandelin]]

It doesn't look like a regexp issue to me: the regression range in comment 17 doesn't include any regexp stuff (nor even JS), and comment 9 says turning off the JIT doesn't avoid the crash.

I can't reproduce on Mac, so I assume this is Windows only. I'll try there and see if I can reproduce it at least. But if I'm right that it's not JS, I probably can't do much more than bisect to a specific changeset.

Comment 32

[David Mandelin [:dmandelin]]

I couldn't reproduce this with either end of the regression range in comment 17 using Vista and the instructions in comment 23. The page seems to load pretty much normally, except that it's apparently still waiting to hear back from google-analytics. But no crash.

Comment 33

[Ruggero Citton]

I can reproduce on Ms Win XP and firefox 3.5.2

Comment 34

[mgueury]

I can reproduce this on 3.5.2 outside of Oracle. Here on XP.

I did this:
-----------
- Downloaded the file oracle.pac above and stored it in the c:\temp\oracle.pac
- Set my automatic proxy URL to c:\temp\oracle.pac
- Then go here: http://people.mozilla.org/~dietrich/ff3crash/ff35-crash.html

-> crash

Comment 35

[mgueury]

I forgot I change the oracle.pac a little for this too happen. (added mozilla.org in the internal URLs)

-> uploaded a new oracle.pac

Comment 38

[David Mandelin [:dmandelin]]

OK, I repro'd this just now on Vista. Instructions in comment 34 worked on the first try. I need to make a debug build in order to see exactly what's crashing, but the stack trace shows it in necko, so I suggest sending this bug over there.

 	necko.dll!nsCacheService::ProcessPendingRequests(nsCacheEntry * entry=0x0524dc40)  Line 1887	C++
>	necko.dll!nsCacheService::CloseDescriptor(nsCacheEntryDescriptor * descriptor=0x0450c920)  Line 1690	C++
 	necko.dll!nsCacheEntryDescriptor::Close()  Line 432 + 0x6 bytes	C++
 	necko.dll!nsCacheEntryDescriptor::~nsCacheEntryDescriptor()  Line 74	C++
 	necko.dll!nsCacheEntryDescriptor::`scalar deleting destructor'()  + 0x8 bytes	C++
 	necko.dll!nsCacheEntryDescriptor::Release()  Line 52 + 0x20 bytes	C++
 	xpcom_core.dll!nsCOMPtr_base::assign_assuming_AddRef(nsISupports * newPtr=0x00000000)  Line 457	C++
 	xpcom_core.dll!nsCOMPtr_base::assign_with_AddRef(nsISupports * rawPtr=0x00000000)  Line 89 + 0x8 bytes	C++
 	necko.dll!nsHttpChannel::CloseCacheEntry(int doomOnFailure=1)  Line 2423	C++
 	necko.dll!nsHttpChannel::OnStopRequest(nsIRequest * request=0x00000000, nsISupports * ctxt=0x00000000, unsigned int status=2152398911)  Line 5219	C++
 	necko.dll!nsInputStreamPump::OnStateStop()  Line 577	C++
 	necko.dll!nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream * stream=0x0424efc8)  Line 402	C++
 	xpcom_core.dll!nsInputStreamReadyEvent::Run()  Line 113	C++
 	xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=1, int * result=0x0022f740)  Line 527 + 0x6 bytes	C++
 	xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00000001, int mayWait=1)  Line 230 + 0xd bytes	C++
 	gkwidget.dll!nsBaseAppShell::Run()  Line 170 + 0x9 bytes	C++
 	tkitcmps.dll!nsAppStartup::Run()  Line 194	C++
 	xul.dll!XRE_main(int argc=3, char * * argv=0x01f1d2f0, const nsXREAppData * aAppData=0x01f1ca40)  Line 3394	C++
 	firefox.exe!NS_internal_main(int argc=3, char * * argv=0x01f1d2f0)  Line 157	C++
 	firefox.exe!wmain(int argc=32625392, wchar_t * * argv=0x01f37020)  Line 112	C++
 	firefox.exe!__tmainCRTStartup()  Line 591 + 0x19 bytes	C
 	kernel32.dll!775e4911() 	
 	[Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]	
 	ntdll.dll!7780e4b6() 	
 	ntdll.dll!7780e489()

Comment 39

[Blake Kaplan (:mrbkap) (inactive)]

Bjarne, could you take a look at this?

Comment 40

[Bjarne (:bjarne)]

Yup...  can also be reproduced on Linux using the steps in comment #34.

Comment 41

[Bjarne (:bjarne)]

Mmmm..  not sure about reproducing this on Linux anymore as my crash could be caused by odd http-settings. Could anyone able to reproduce this on Vista/XP dump their network.http.* settings from about:config?

Comment 42

[mgueury]

Since I am not able to copy/paste all the settings, I will upload an screenshot.

Comment 43

[mgueury]

Attachment: about config network.http.*

Comment 44

[Bjarne (:bjarne)]

Thanks.

Comment 45

[Bjarne (:bjarne)]

Attachment: Replacement for proper backout of 3c7adc8fdcb4

(In reply to comment #31)
> It doesn't look like a regexp issue to me: the regression range in comment 17
> doesn't include any regexp stuff (nor even JS)

Not entirely correct - see 3c7adc8fdcb4

I was unable to back out 3c7adc8fdcb4 properly (my hg-fu is pretty weak, and lots of changes has happened to the file in question since then) so for illustration I attach the simple hack I did to eliminate the effects of 3c7adc8fdcb4. With this change, I can load and reload the url from comment #34 (with pac-file installed of-course) any number of times without crashing.

Thus, it looks like shavers guts in comment #29 were right. It would be nice if someone could try out the patch on vista/xp and report back. If appropriate, feel free to reassign and move bug to the proper component.

Comment 46

[David Mandelin [:dmandelin]]

I will try your patch on Vista. In the meantime, can you give me the hg.mozilla.org URL for the changeset you are talking about? I didn't find a 3c7adc8fdcb4 in tracemonkey or mozilla-central.

Comment 47

[Bjarne (:bjarne)]

See regression range from comment #17

Comment 48

[David Mandelin [:dmandelin]]

Do you mean this?

http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?startdate=May+22+2009&enddate=May+23+2009

I see no 3c7adc8fdcb4 there.

Comment 49

[David Mandelin [:dmandelin]]

The patch in comment 45 makes it work for me.

Comment 50

[Bjarne (:bjarne)]

(In reply to comment #48)
> Do you mean this?
> 
> http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?startdate=May+22+2009&enddate=May+23+2009
> 
> I see no 3c7adc8fdcb4 there.

Weird... item #11 from the top on my list reads

[e-mail]
Fri May 22 12:52:07 2009 -0700	3c7adc8fdcb4	Ben Newman — Bug 484107 - XPCSafeJSObjectWrapper allows regexp variables to be clobbered. r=mrbkap+sr=brendan

(In reply to comment #49)
> The patch in comment 45 makes it work for me.

Good! :)

Then I guess someone with the right authority and hg-knowledge can do whatever necessary to re-fix or back out 3c7adc8fdcb4 in order to resolve this issue? On Linux, I can reproduce crashes in 2-3 different locations depending on how I load the URL, although none of them similar to the one in e.g. comment #38. The crashes I see all look like some type of heap-corruption, most likely freeing some pointer twice.

Comment 51

[Ben Newman (:bnewman) (:benjamn)]

(In reply to comment #50)
> Then I guess someone with the right authority and hg-knowledge can do whatever
> necessary to re-fix or back out 3c7adc8fdcb4 in order to resolve this issue?

I can take care of it.  Just backing it out doesn't count as a resolution, of course.  I'm still trying to reproduce the crash, so that I can get some insight into the source of the corruption.

Comment 52

[Bjarne (:bjarne)]

I did not manage to reproduce the original crash - my result from comment #40 was caused by some experimental http-settings left over in my test-profile.

Fwiw, I reproduced 2-3 different types of crash using ctrl-r to reload the page.

Comment 53

[Ben Newman (:bnewman) (:benjamn)]

Attachment: Patch to prevent double-freeing cx->regExpStatics.moreParens (explains the problem and appears to fix it).

Analysis: cx->regExpStatics.moreParens was getting freed while cx->regExpStatics resided in the local variable statics (in CallWithoutStatics). Then, after js_RestoreRegExpStatics restored a copy of the freed pointer, the pointer was eventually freed again, i.e. twice.  Heap corruption for the loss.

Comment 54

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 396969 [details] [diff] [review]
Patch to prevent double-freeing cx->regExpStatics.moreParens (explains the problem and appears to fix it).

>diff --git a/js/src/jsregexp.cpp b/js/src/jsregexp.cpp
>+js_SaveAndClearRegExpStatics(JSContext *cx, JSRegExpStatics *statics,
>+                             JSTempValueRooter *tvr)
> {
>   *statics = cx->regExpStatics;
>   JS_PUSH_TEMP_ROOT_STRING(cx, statics->input, tvr);
>+  /* Prevent JS_ClearRegExpStatics from freeing moreParens, since we've only
>+   * moved it elsewhere (into statics->moreParens). */

Nit: Major comments in the JS engine are formatted as:

/*
 * comment text
 * here.
 */

Please also add a newline above the /*.

This needs to land everywhere.

Comment 55

[Ben Newman (:bnewman) (:benjamn)]

Attachment: backport to 1.9.1

Only difference was that I had to use JS_free instead of JSContext::free in JS_ClearRegExpStatics.

Comment 56

[Jeff Walden [:Waldo]]

(In reply to comment #53)
> Patch to prevent double-freeing

Ruh-roh!

Comment 57

[Ben Newman (:bnewman) (:benjamn)]

Attachment: backport to 1.9.2

Comment 58

[Ben Newman (:bnewman) (:benjamn)]

Attachment: backport to 1.9.0

Two caveats.  This is a patch generated by hg for the CVS tree, so it won't necessarily apply cleanly to a plain CVS checkout.  Also, it assumes that the 1.9.0-backported patch for bug 484107 actually landed, which it didn't.

Comment 59

[Brendan Eich [:brendan]]

Comment on attachment 396969 [details] [diff] [review]
Patch to prevent double-freeing cx->regExpStatics.moreParens (explains the problem and appears to fix it).

>diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp
>--- a/js/src/jsapi.cpp
>+++ b/js/src/jsapi.cpp
>@@ -5693,16 +5693,20 @@ JS_ClearRegExpStatics(JSContext *cx)
> 
>     /* No locking required, cx is thread-private and input must be live. */
>     res = &cx->regExpStatics;
>     res->input = NULL;
>     res->multiline = JS_FALSE;
>     res->parenCount = 0;
>     res->lastMatch = res->lastParen = js_EmptySubString;
>     res->leftContext = res->rightContext = js_EmptySubString;
>+    if (res->moreParens) {
>+      cx->free(res->moreParens);
>+      res->moreParens = NULL;

c-basic-offset: 4 in js*.{cpp,h}.

Fix that and the nits mrbkap caught and sr=me.

/be

Comment 60

[Ben Newman (:bnewman) (:benjamn)]

Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/9e16ec20e6fe

(In reply to comment #59)
> (From update of attachment 396969 [details] [diff] [review])
> c-basic-offset: 4 in js*.{cpp,h}.
> 
> Fix that and the nits mrbkap caught and sr=me.
> 
> /be

Caught this in a follow-up commit:
http://hg.mozilla.org/mozilla-central/rev/a2f32d07f9fa

Comment 61

[Bjarne (:bjarne)]

Is this ok now?

Comment 62

[David Mandelin [:dmandelin]]

(In reply to comment #61)
> Is this ok now?

Yes, it is fixed in m-c. I set the assignee to bnewman so you don't have to think about it any more. :-)

Comment 63

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

bnewman: JS doesn't require SR, so this is good to request branch approval, unless you specifically wanted jst to look at something.

Comment 64

[Ben Newman (:bnewman) (:benjamn)]

(In reply to comment #63)
> bnewman: JS doesn't require SR, so this is good to request branch approval,
> unless you specifically wanted jst to look at something.

Requested, thanks much!

Comment 65

[Samuel Sidler (old account; do not CC)]

This needs to land on 1.9.2 before we take it on 1.9.1 or 1.9.0.

Comment 66

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 397114 [details] [diff] [review]
backport to 1.9.2

Benjamin: please land on 1.9.2! a=shaver

Comment 67

[Ben Newman (:bnewman) (:benjamn)]

(In reply to comment #66)
> (From update of attachment 397114 [details] [diff] [review])
> Benjamin: please land on 1.9.2! a=shaver

Done:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/f54252526942

Comment 68

[Samuel Sidler (old account; do not CC)]

(And don't forget to set the status1.9.2 flag when you do.)

Comment 69

[Daniel Veditz [:dveditz]]

Comment on attachment 397108 [details] [diff] [review]
backport to 1.9.1

Approved for 1.9.1.4, a=dveditz

Comment 70

[Daniel Veditz [:dveditz]]

Comment on attachment 397150 [details] [diff] [review]
backport to 1.9.0

Approved for 1.9.0.15, a=dveditz

Comment 71

[Ben Newman (:bnewman) (:benjamn)]

Attachment: Patch incorporating approved but unlanded 484107 changes.

Since bug 484107 never actually landed on 1.9.0, FF3.0 fortunately was never subject to this crash.  This patch kills two birds (bug 484107 and bug 500644) with one stone.

Posting here because I don't have CVS commit access.  JST has volunteered to land.

Comment 72

[Ben Newman (:bnewman) (:benjamn)]

Landed backport to 1.9.1.4:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0878db40d356

Comment 73

[Daniel Veditz [:dveditz]]

Comment on attachment 400622 [details] [diff] [review]
Patch incorporating approved but unlanded 484107 changes.

Approved for 1.9.0.15, a=dveditz

Comment 74

[Daniel Veditz [:dveditz]]

Comment on attachment 397150 [details] [diff] [review]
backport to 1.9.0

This is incorporated into the other patch, doesn't need approval anymore.

Comment 75

[Daniel Veditz [:dveditz]]

Looks like jst landed this last week, shortly after Ben's comment 72

Comment 76

[Al Billings [:abillings - ex-MoCo]]

(In reply to comment #34)
> I can reproduce this on 3.5.2 outside of Oracle. Here on XP.
> 
> I did this:
> -----------
> - Downloaded the file oracle.pac above and stored it in the c:\temp\oracle.pac
> - Set my automatic proxy URL to c:\temp\oracle.pac
> - Then go here: http://people.mozilla.org/~dietrich/ff3crash/ff35-crash.html
> 
> -> crash

With 1.9.1.4, I was able to repro the crash and verify the fix for 1.9.1.5 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.4pre) Gecko/20090922 Shiretoko/3.5.4pre (.NET CLR 3.5.30729). 

Using 1.9.0.14, I was not able to reproduce the crash. With both 1.9.0.14 and the nightly 1.9.0.15pre build, the page in Dietrich's directory loads without crashing (and the pac file is clearly working as I cannot go to google.com, for example).

Comment 77

[Al Billings [:abillings - ex-MoCo]]

Based on comment 71, I'm going to assume that I'm seeing correct behavior but this bug cannot be verified on 1.9.0.

Comment 78

[Ben Newman (:bnewman) (:benjamn)]

(In reply to comment #77)
> Based on comment 71, I'm going to assume that I'm seeing correct behavior but
> this bug cannot be verified on 1.9.0.

That's correct.  Thanks for the verifications!

Comment 79

[mgueury]

I just want to say thank you. It is working fine with Firefox 3.5.4.