PAC: crash on invalid entries

RESOLVED FIXED in mozilla1.8alpha3

Status

()

--
critical
RESOLVED FIXED
15 years ago
14 years ago

People

(Reporter: jmccabe, Assigned: Biesinger)

Tracking

({crash, regression})

Trunk
mozilla1.8alpha3
x86
Linux
crash, regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

15 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a3) Gecko/20040726
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a3) Gecko/20040726

My .pac file lists multiple proxies (failover proxies to use if the first is
unavailable).

Mozilla crashes almost immediately after beginning to display content.

Simplifying the .pac so that only a single Proxy is returned makes the crash go
away.

Reproducible: Always
Steps to Reproduce:
1. Use .pac file that looks something like:
 function FindProxyForURL(url, host)
    {return "PROXY 192.168.0.1:3128; 192.168.0.2:3128";} 

(adjust at least the first IP to point to a Proxy in your network)
2. Access a site
3. Restart Mozilla (since it will have just crashed)

Actual Results:  
Crash.

The stack trace ends with:
#0  0x400a63df in PR_AtomicDecrement () from /usr/local/mozilla/libnspr4.so
#1  0x40c29bf3 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#2  0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#3  0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#4  0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#5  0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#6  0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#7  0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#8  0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#9  0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#10 0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#11 0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#12 0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#13 0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#14 0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#15 0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#16 0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#17 0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#18 0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#19 0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#20 0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so
#21 0x40c29c24 in NSGetModule () from /usr/local/mozilla/components/libnecko.so

Hard to say where the stack began. I followed it as far as:

#73184 0x40c29c24 in NSGetModule ()
   from /usr/local/mozilla/components/libnecko.so
#73185 0x40c29c24 in NSGetModule ()
   from /usr/local/mozilla/components/libnecko.so
#73186 0x40c29c24 in NSGetModule ()
   from /usr/local/mozilla/components/libnecko.so
#73187 0x40c29c24 in NSGetModule ()
   from /usr/local/mozilla/components/libnecko.so
#73188 0x40c29c24 in NSGetModule ()
   from /usr/local/mozilla/components/libnecko.so
#73189 0x40c29c24 in NSGetModule ()
   from /usr/local/mozilla/components/libnecko.so
(Reporter)

Updated

15 years ago
Summary: Crash is .pac file lists multiple proxies → Crash if .pac file lists multiple proxies

Updated

15 years ago
Keywords: crash
the stack is useless, you created in with a stripped build. can you send a
talkback report, or get a stack using a debug, or at least non-stripped, build?

>#0  0x400a63df in PR_AtomicDecrement () from /usr/local/mozilla/libnspr4.so
>#1  0x40c29bf3 in NSGetModule () from /usr/local/mozilla/components/libnecko.so

*maybe* this means that a component with threadsafe addref/release was released
one time too much...
(Reporter)

Comment 2

15 years ago
Rebuilding with --enable-debug. Will post stack from this build. You have any
other favorite build flags I should use?
(Reporter)

Comment 3

15 years ago
Rebuilt with --enable-debug. The core file isn't much more useful.

#0  0x400c38e2 in PR_AtomicDecrement () from /usr/local/mozilla/libnspr4.so
#1  0x40f1e4f1 in nsCString::operator=(char const*) ()
   from /usr/local/mozilla/components/libnecko.so
#2  0x40f21b41 in nsProxyInfo::~nsProxyInfo() ()
   from /usr/local/mozilla/components/libnecko.so
#3  0x40f1e52c in nsCString::operator=(char const*) ()
   from /usr/local/mozilla/components/libnecko.so
#4  0x40f21b41 in nsProxyInfo::~nsProxyInfo() ()
   from /usr/local/mozilla/components/libnecko.so
#5  0x40f1e52c in nsCString::operator=(char const*) ()
   from /usr/local/mozilla/components/libnecko.so


A new observation of the failure:

The crash does not occur if the .pac file is more strictly compliant with the
format described at
http://wp.netscape.com/eng/mozilla/2.0/relnotes/demo/proxy-live.html:

CRASH:
{return "PROXY 192.168.0.1:3128; 192.168.0.2:3128";} 

NO CRASH:
{return "PROXY 192.168.0.1:3128; PROXY 192.168.0.2:3128";} 

Unsure where the bogus syntax came from (I'm having trouble finding other
examples of it).

This bug is likely bogus since the .pac file was incorrectly written. Still -
It'd be nice if the browser didn't crash when Bad Things happen.
-> me

I could reproduce the bug. I saw an infinite recursion there, with
PR_AtomicDecrement at the top and only nsProxyInfo::Release below...

probably the additional PROXY there confuses the PAC parser and possibly creates
a loop in the nsProxyInfo linked list
Assignee: darin → cbiesinger
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 5

15 years ago
(In reply to comment #4)
> probably the additional PROXY there confuses the PAC parser and possibly creates
> a loop in the nsProxyInfo linked list

Actually it's the lack of the additional PROXY that seems to cause the loop.

CRASH:
{return "PROXY 192.168.0.1:3128; 192.168.0.2:3128";} 

NO CRASH:
{return "PROXY 192.168.0.1:3128; PROXY 192.168.0.2:3128";} 
Created attachment 154502 [details] [diff] [review]
patch

well... *result was never set to null... so it still contained the previous
proxyinfo... -> the linked list had a loop.
Attachment #154502 - Flags: superreview?(darin)
Attachment #154502 - Flags: review?(darin)
(Reporter)

Comment 7

15 years ago
Confirmed that the patch makes the crash go away when a bogus .pac file lists
multiple proxy servers.

You rock
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8alpha3
hmm, bug 224237 removed this line... and I reviewed that patch...
Keywords: regression

Comment 9

15 years ago
Comment on attachment 154502 [details] [diff] [review]
patch

r+sr=darin

thanks biesi!  do we want this on other branches?
Attachment #154502 - Flags: superreview?(darin)
Attachment #154502 - Flags: superreview+
Attachment #154502 - Flags: review?(darin)
Attachment #154502 - Flags: review+
It looks like this broke after 1.7 branched; so, all branches should be fine, I
think.

Checking in netwerk/base/src/nsProtocolProxyService.cpp;
/cvsroot/mozilla/netwerk/base/src/nsProtocolProxyService.cpp,v  <-- 
nsProtocolProxyService.cpp
new revision: 1.57; previous revision: 1.56
done
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 11

14 years ago
So does the fix support this format, or simply prevent the crash?

Comment 12

14 years ago
> So does the fix support this format, or simply prevent the crash?

I believe it means that we'd ignore the incorrectly specified server, and
continue parsing after the next ';'

Comment 13

14 years ago
Oh, I see what you mean now.
Summary: Crash if .pac file lists multiple proxies → PAC: crash on invalid entries
You need to log in before you can comment on or make changes to this bug.