305260 - Returning empty proxy string in proxy.pac file crashes Firefox. [@ nsHttpConnectionInfo::nsHttpConnectionInfo]

Status: RESOLVED WORKSFORME

(Core :: Networking, defect)

Description

[Chris Lawson]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0; .NET CLR 1.1.4322)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.10) Gecko/20050716 

Returning bad or incomplete values in proxy.pac can crash Firefox.  Is this 
also a potential attack vector for a buffer overflow?

Reproducible: Always

Steps to Reproduce:
1.  Configure a proxy.pac similar to the example below.  Notice the last line 
just returns "PROXY" instead of something like "PROXY 192.168.123.56"

// Example Proxy.Pac
function FindProxyForURL(url, host)
{
   // If It's An Address Inside Firewall - Send Direct 
   if (   isInNet(host, "192.168.1.0", "255.255.255.0") )
       {   return "DIRECT";   }

   // Otherwise, We Send Them Direct.  
   return  "PROXY ";

}










Actual Results:  
Firefox crashes.  Ironically the error report gets submitted successfully via 
our proxy server despite it not being configured for it.  So it also appears 
that Firefox is caching proxy information somewhere.

Expected Results:  
Report an error reaching the proxy.  Same response if an unreachable proxy were 
specified.

Comment 1

[timeless]

talkback uses ie settings. can you please run talkback.exe and copy the 
incident id to this bug?

Comment 2

[Frank Wein [:mcsmurf]]

Stacktrace (with FF 1.0.4):
Trigger Reason	Access violation

MSVCRT.DLL + 0x1d0b (0x78001d0b)
nsHttpConnectionInfo::nsHttpConnectionInfo 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/netwerk/protocol/http/src/nsHttpConnectionInfo.h,
line 54]
nsHttpHandler::NewProxiedChannel 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/netwerk/protocol/http/src/nsHttpHandler.cpp,
line 1418]
nsIOService::NewChannelFromURI 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/netwerk/base/src/nsIOService.cpp,
line 475]
NS_NewChannel  [../../../dist/include/necko/nsNetUtil.h, line 166]
nsDocShell::DoURILoad 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/docshell/base/nsDocShell.cpp,
line 5762]
nsDocShell::InternalLoad 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/docshell/base/nsDocShell.cpp,
line 5678]
nsDocShell::LoadURI 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/docshell/base/nsDocShell.cpp,
line 734]
nsDocShell::LoadURI 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/docshell/base/nsDocShell.cpp,
line 2761]
XPTC_InvokeByIndex 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp,
line 102]
XPCWrappedNative::CallMethod 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,
line 2034]
XPC_WN_CallMethod 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1781]
js_Invoke 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c,
line 955]
js_Interpret 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c,
line 2999]
js_Invoke 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c,
line 972]
js_Interpret 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c,
line 2999]
js_Invoke 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c,
line 972]
js_InternalInvoke 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c,
line 1049]
JS_CallFunctionValue 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/js/src/jsapi.c,
line 3698]
nsJSContext::CallEventHandler 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 1297]
nsJSEventListener::HandleEvent 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/dom/src/events/nsJSEventListener.cpp,
line 184]
nsEventListenerManager::HandleEventSubType 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1436]
nsEventListenerManager::HandleEvent 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1516]
GlobalWindowImpl::HandleDOMEvent 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/dom/src/base/nsGlobalWindow.cpp,
line 927]
DocumentViewerImpl::LoadComplete 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/content/base/src/nsDocumentViewer.cpp,
line 917]
nsDocShell::EndPageLoad 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/docshell/base/nsDocShell.cpp,
line 4602]
nsWebShell::EndPageLoad 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/docshell/base/nsWebShell.cpp,
line 760]
nsDocShell::OnStateChange 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/docshell/base/nsDocShell.cpp,
line 4536]
nsDocLoaderImpl::FireOnStateChange 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/uriloader/base/nsDocLoader.cpp,
line 1252]
nsDocLoaderImpl::doStopDocumentLoad 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/uriloader/base/nsDocLoader.cpp,
line 873]
nsDocLoaderImpl::OnStopRequest 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/uriloader/base/nsDocLoader.cpp,
line 701]
nsLoadGroup::RemoveRequest 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/netwerk/base/src/nsLoadGroup.cpp,
line 695]
imgRequestProxy::RemoveFromLoadGroup 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/modules/libpr0n/src/imgRequestProxy.cpp,
line 161]
imgRequestProxy::OnStopRequest 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/modules/libpr0n/src/imgRequestProxy.cpp,
line 448]

Comment 3

[Frank Wein [:mcsmurf]]

BTW: In Trunk this doesn't crash anymore but leads to interesting behaviour.
When you start SeaMonkey with this PAC activated, you can surf to one
website/host. All HTTP requests after that go to that host (so only websites
from this website will then work, for other websites you'll see HTTP 404 and 403
errors returning from webserver). If you active it after you started it, it'll
"remember" the first host you surfed to, too.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

hm... the code around
http://lxr.mozilla.org/seamonkey/source/netwerk/base/src/nsProtocolProxyService.cpp#628
makes the proxy info's host an empty string

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

er, actually, it just doesn't modify the host (but it's an nsCString, so it's
still the empty string)

On the branch, it ends up as null; and
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/netwerk/protocol/http/src/nsHttpConnectionInfo.h&mark=80&rev=AVIARY_1_0_1_20050124_BRANCH#70
tries to construct an nsDependentCString from that and crashes. Not a buffer
overflow, just a normal NULL pointer dereference.

There's this nice comment in nsProtocolProxyService:
460                                // YES, it is ok to specify a null proxy host.

Comment 6

[Boris Zbarsky [:bzbarsky]]

So this is worksforme unless some 1.7 branch maintainers want this, right?

Comment 7

[Wayne Mery (:wsmwk)]

bz, ok to close?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Yes.