Closed Bug 533529 Opened 12 years ago Closed 12 years ago

Crash [@ nsAuthSSPI::GetNextToken]


(Core :: Networking: HTTP, defect)

1.9.2 Branch
Windows XP
Not set



Tracking Status
status1.9.2 --- final-fixed


(Reporter: cbook, Assigned: jimm)




(Keywords: crash, verified1.9.2, Whiteboard: [crashkill])

Crash Data


(3 files)

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b6pre) Gecko/20091207
Namoroka/3.6b6pre (debug Build) and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b6pre) Gecko/20091208 Namoroka/3.6b6pre (opt)

-> Steps to reproduce:
--> You get a Auth-Popup 
--> Click cancel (we of course have no login there :)
---> Crashes

(45c.d80): Access violation - code c0000005 (!!! second chance !!!)
eax=00000000 ebx=7ffd9000 ecx=00000000 edx=0764d734 esi=0101374c edi=000001fd
eip=01516f53 esp=0012e35c ebp=0012e488 iopl=0         nv up ei pl nz ac po nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00000212
*** WARNING: Unable to verify checksum for c:\work\mozilla\builds\1.9.2\mozilla\
01516f53 8b4818          mov     ecx,dword ptr [eax+18h] ds:0023:00000018=??????

Exploitability Classification: PROBABLY_EXPLOITABLE
Recommended Bug Title: Probably Exploitable - Data from Faulting Address control
s Code Flow starting at auth!nsAuthSSPI::GetNextToken+0x00000000000001d3 (Hash=0

The data from the faulting address is later used as the target for a branch.
ChildEBP RetAddr
0012e488 01512a98 auth!nsAuthSSPI::GetNextToken+0x1d3
0012e4c8 02dd2f64 auth!nsHttpNegotiateAuth::GenerateCredentials+0x1b8
0012e520 02dd40cf necko!nsHttpChannel::GenCredsAndSetEntry+0xb4
0012e6e0 02dd36ea necko!nsHttpChannel::GetCredentialsForChallenge+0x65f
0012e794 02dd507a necko!nsHttpChannel::GetCredentials+0x18a
0012e81c 003204e7 necko!nsHttpChannel::OnAuthCancelled+0x9a
0012e838 039b549b xpcom_core!NS_InvokeByIndex_P+0x27
0012ec08 039c30d6 xpc3250!XPCWrappedNative::CallMethod+0x129b
0012ecd4 0052b61d xpc3250!XPC_WN_CallMethod+0x1c6
0012edb0 0053da52 js3250!js_Invoke+0x87d
0012f474 0052b662 js3250!js_Interpret+0xffc2
0012f540 039aa848 js3250!js_Invoke+0x8c2
0012f8b4 039a1571 xpc3250!nsXPCWrappedJSClass::CallMethod+0x1058
0012f8d4 00320a23 xpc3250!nsXPCWrappedJS::CallMethod+0x41
0012f9a8 003206e6 xpcom_core!PrepareAndDispatch+0x323
0012f9c4 0030d3ea xpcom_core!SharedStub+0x16
0012fa00 00297b83 xpcom_core!nsThread::ProcessNextEvent+0x1fa
0012fa1c 02a6d1ed xpcom_core!NS_ProcessNextEvent_P+0x53
0012fa30 036c42db gkwidget!nsBaseAppShell::Run+0x5d
0012fa44 1000c302 tkitcmps!nsAppStartup::Run+0x6b
Flags: blocking1.9.2?
note: opt builds crash:

It seems this Crash is somehow related to Bug 531425 - since its a opt build crahs with the same signatures , but with my steps to reproduce debug builds and opt builds crash after the fix from jim was checked in.
Looks like we make a somewhat flawed assumption in nsHttpChannel's OnAuthCancelled where we assume the auth module has already been initialized. In the use of AuthSSPI, we don't init the module until credentials are present. In the case where the user enters something into the auth prompt, AuthSSPI's Init is called via nsHttpChannel's OnAuthAvailable -> GenCredsAndSetEntry. OnAuthCancelled however calls into GetNextToken via GetCredentials. Other auth modules deal with this gracefully, AuthSSPI however never expected it.

This patch addresses this by exiting from GetNextToken if initialization hasn't occurred yet.
Assignee: nobody → jmathies
Attachment #416779 - Flags: review?(cbiesinger)
Also a note for driver - this should block 1.9.2.
Summary: Probably Exploitable - Data from Faulting Address controls Code Flow starting at auth!nsAuthSSPI::GetNextToken+0x00000000000001d3 → Crash [@ nsAuthSSPI::GetNextToken]
Blocks: 520607
This was a regression from bug 520607 where we added support for prompting of user credentials when using nsAuthSSPI. The module didn't expect a call into GetNextToken before initialization, which we occasionally do when someone cancels the auth. credentials prompt.
Flags: blocking1.9.2? → blocking1.9.2+
Comment on attachment 416779 [details] [diff] [review]
GetNextToken patch v.1

Attachment #416779 - Flags: review?(cbiesinger) → review+

Jim, do you think you can create a testcase for this?
Closed: 12 years ago
Resolution: --- → FIXED
I need to reopen this. Testing on a related bug this patch has messed up single sign on. The problem is that the ctx isn't initialized until later, I need to be checking the context instead. Will have an update here hopefully in a little bit.
Resolution: FIXED → ---
Do we not have single sign on tests?
(In reply to comment #9)
> Do we not have single sign on tests?

I don't believe we have the ability to test ntlm auth as it requires a windows web server running that our test infrastructure can hit. We should have something like this, both a web and proxy server (like IIS and ms's proxy) to automate testing this code. I can file a bug to see what's possible.
Ok, this updates the last patch to look at the credential handle values instead of the context. I ran this through a set of local tests - ntlm auth prompting is working, single sign-on now works correctly, and the crash on cancel is still prevented.
Attachment #417955 - Flags: review?(bzbarsky)
Attachment #417955 - Flags: review?(bzbarsky) → review+
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
I can add a Mozmill test for this particular problem to our little and tiny crash test suite which only has 1 crash test in there ATM. Should be a simple thing. I will put it on my list.
Target Milestone: --- → mozilla1.9.3a1
We could use this temporary Mozmill crash test until we have an own server available. I can push it to our repository once this bug has been made public.
Flags: in-testsuite?
Verified fixed on trunk and 1.9.2 with:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20091217 Minefield/3.7a1pre (.NET CLR 3.5.30729)

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b6pre) Gecko/20091217 Namoroka/3.6b6pre (.NET CLR 3.5.30729)
Keywords: verified1.9.2
Crash Signature: [@ nsAuthSSPI::GetNextToken]
Group: core-security
You need to log in before you can comment on or make changes to this bug.