User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.2) Gecko/20090803 Fedora/3.5.2-2.fc11 Firefox/3.5.2 Build Identifier: 3.0b3 / 20090715140311 The UC Berkeley central email system recently enabled GSSAPI support for SMTP, IMAP, and POP. Thunderbird 2.0.0.22 on Windows XP when joined to AD works fine; however, under Vista or Windows 7 (and joined to AD), Thunderbird crashes when attempting to use SSPI. Thunderbird 2, by default, will always attempt to use SSPI during SMTP, and we've received widespread reports across campus of Thunderbird crashing when attempting to send email. Setting network.auth-use-sspi to false returns Thunderbird to a stable state. We have been unable to get Talkback data for Thunderbird 2, so we've replicated the problem with Thunderbird 3.0b3 and Breakpad, which appears to also crash under SSPI. This bug may be a duplicate of bug 505971. However, since this behaviour is widespread in our environment, and Thunderbird immediately crashes with SMTP or IMAP when joined to AD on Vista or later and SSPI is enabled it seems worthwhile to file a separate bug report. Reproducible: Always Steps to Reproduce: 1. Join Windows Vista (or later) machine to CAMPUS.BERKELEY.EDU AD 2. Configure Thunderbird client to use calmail.berkeley.edu (which will advertise AUTH=GSSAPI for SSL/TLS connections). 3. Attempt to send email with Thunderbird 2 (or under Thunderbird 3, check "Use secure authentication" for SMTP setup) or retrieve email with "Use secure authentication" checked for IMAP/POP Actual Results: Thunderbird crashes. Expected Results: Thunderbird performs GSSAPI SASL negotiation and sends email or retrieves email. http://crash-stats.mozilla.com/report/index/40d6d04c-6423-47b1-898d-6a8a62090820 is a crash of 3.0b3 under Vista using SSPI to login to IMAP.

so, your description indicates it should be that bug, but your crash says it's totally unrelated. Signature strcat UUID 40d6d04c-6423-47b1-898d-6a8a62090820 Time 2009-08-20 16:55:07.232911 Uptime 3 Product Thunderbird Version 3.0b3 Build ID 20090715140311 Branch 1.9.1 OS Windows NT OS Version 6.0.6002 Service Pack 2 CPU x86 CPU Info GenuineIntel family 15 model 6 stepping 4 Crash Reason EXCEPTION_ACCESS_VIOLATION Crash Address 0x0 User Comments using sspi to log into IMAP Processor Notes Crashing Thread Frame Module Signature [Expand] Source 0 mozcrt19.dll strcat strcat.asm:178 1 thunderbird.exe nsImapFlagAndUidState::AddUidCustomFlagPair mailnews/imap/src/nsImapFlagAndUidState.cpp:365 2 thunderbird.exe nsImapServerResponseParser::msg_fetch mailnews/imap/src/nsImapServerResponseParser.cpp:1385 3 thunderbird.exe nsImapServerResponseParser::response_data mailnews/imap/src/nsImapServerResponseParser.cpp:756 4 thunderbird.exe nsImapServerResponseParser::ParseIMAPServerResponse mailnews/imap/src/nsImapServerResponseParser.cpp:243 5 thunderbird.exe nsImapProtocol::ParseIMAPandCheckForNewMail mailnews/imap/src/nsImapProtocol.cpp:1853 6 thunderbird.exe nsImapProtocol::FetchMessage mailnews/imap/src/nsImapProtocol.cpp:3416 7 thunderbird.exe nsImapProtocol::ProcessMailboxUpdate mailnews/imap/src/nsImapProtocol.cpp:3917 8 thunderbird.exe nsImapProtocol::ProcessSelectedStateURL mailnews/imap/src/nsImapProtocol.cpp:2819 9 thunderbird.exe nsImapProtocol::ProcessCurrentURL mailnews/imap/src/nsImapProtocol.cpp:1709 10 thunderbird.exe nsImapProtocol::ImapThreadMainLoop mailnews/imap/src/nsImapProtocol.cpp:1360 11 thunderbird.exe nsImapProtocol::Run mailnews/imap/src/nsImapProtocol.cpp:1059 12 xpcom_core.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:510 13 xpcom_core.dll NS_ProcessNextEvent_P nsThreadUtils.cpp:227 14 xpcom_core.dll nsThread::ThreadFunc xpcom/threads/nsThread.cpp:254 15 nspr4.dll _PR_NativeRunThread nsprpub/pr/src/threads/combined/pruthr.c:426 16 nspr4.dll pr_root nsprpub/pr/src/md/windows/w95thred.c:122 17 mozcrt19.dll _callthreadstartex threadex.c:348 18 mozcrt19.dll _threadstartex threadex.c:326 19 kernel32.dll BaseThreadInitThunk 20 ntdll.dll __RtlUserThreadStart 21 ntdll.dll _RtlUserThreadStart

Yes, I agree that the backtrace doesn't match the bug description; however if the bug is the same as bug 505971, I would assume the use of SSPI, in and of itself, could cause Thunderbird to become unstable and blow up at some later point. I don't have immediate access to a Vista machine, but I'll try to get some additional crash reports submitted in the next few days.

I got a crash report generated under Windows Server 2008 R2 (equivalent to Windows 7) when trying to use SMTP with SSPI enabled and while logged into an AD account: http://crash-stats.mozilla.com:80/report/index/e148a46f-4282-42fb-8604-6424f2090821

This crash occurred when using TB 3b3 under Win Server 2008 R2 when gssapi-enabled imap server was being accessed while logged into an AD account with network.auth-use-sspi=true: http://crash-stats.mozilla.com:80/report/index/7fb38a9e-a2f6-4082-8a2f-df43d2090821?p=1

Karl your problem is described in bug 505971

marking blocker - I think bug 505971 is the same but I'll do the work here for now.

With much help from Karl (thx!) I've got an environment to reproduce this bug. The issue seems to be in the Microsoft secur32.dll (which might help explain why this bug seems to be new to Vista/Windows 7). When we call DecryptMessage on a token, we get back a success error code and a valid unwrapped token len, but trying to free the resulting unwrapped token crashes. According to the comments in the code, we're only unwrapping the token to verify that the signature was correct (we're not doing security layers). Since we're not using the unwrapped token, and don't care what it is, commenting out the freeing of the memory makes things work fine. I suppose it's possible that we should be freeing the memory differently, but an other possibility is that the arguments we're passing to DecryptMessage might be exposing a bug in secur32.dll. I suppose it's also remotely possible that we shouldn't be using secur32.dll - this code is fairly old. Cc'ing Simon who might just know off the top of his head what's going wrong. Here's the call to DecryptMessage: http://mxr.mozilla.org/comm-central/source/mozilla/extensions/auth/nsAuthSSPI.cpp#407 and here's the free call: http://mxr.mozilla.org/comm-central/source/mozilla/extensions/auth/nsAuthSASL.cpp#135 I'm marking security sensitive for now, just because it's a potential heap corruption, though I don't know if it's exploitable.

In the MSDN sample code, they don't say anything about having the free the result data, but I don't know if that's just assumed or not.

they do say that the data is decrypted in place, from the input buffer, which implies we wouldn't have to free it. http://msdn.microsoft.com/en-us/library/aa375215%28VS.85%29.aspx Our code looks to be straight from the sample code, except that the sample code doesn't free the memory.

(In reply to comment #7) > With much help from Karl (thx!) I've got an environment to reproduce this bug. > The issue seems to be in the Microsoft secur32.dll (which might help explain > why this bug seems to be new to Vista/Windows 7). Therefore seems not dupe at all of bug 505971.

why do you say that? Isn't that SSPI GSSAPI as well? Or are you saying you're having the issue with XP?

(In reply to comment #11) > Or are you saying you're having the issue with XP? Exactly, there crash reports from Windows XP, tested on Windows 7 and XP.

I have no technical reason to believe that the issue doesn't happen on XP, other than some folks say they haven't seen it on XP. If you can do your own builds, I can tell you what line to comment out and see if the problem goes away on XP.

If you are talking about exactly this bug, I'm not aware any problems such as these described by reporter. Only those reported in bug 505971. Still I can make my own builds (at least give a try)

the general problem is a crash after doing SSPI GSSAPI authentication due to heap corruption. bug 505971 sounds like it matches. Given the nature of heap corruption, and this bug in particular, I would expect the actual crash site to differ between builds and OS's. I haven't tried this myself, but a simple fix would be to find this code 414 if (SEC_SUCCESS(rc)) { 415 *outToken = ib[1].pvBuffer; 416 *outTokenLen = ib[1].cbBuffer; and just change the *outToken = ... line to *outToken = NULL; which is here: http://mxr.mozilla.org/comm-central/source/mozilla/extensions/auth/nsAuthSSPI.cpp#407

I think this affects Firefox, in theory, at least, since the code is shared with Firefox. We very much would like this for the next beta of TB 3, which has code freeze in about 2 weeks. Someone other than me should probably figure out if it affects Firefox in actual fact, but I think this fix should go into whatever versions of gecko Thunderbird ships on, at least. Since I only have the version of gecko that Thunderbird uses, once this is reviewed, it would be helpful if someone else could land it on the trunk...

Comment on attachment 397375 [details] [diff] [review] proposed fix 1.9.1.3 is frozen.

Comment on attachment 397375 [details] [diff] [review] proposed fix > > else > nsMemory::Free(ib[1].pvBuffer); This should free ib[0].pvBuffer (which we've allocated), not ib[1].pvBuffer, which may not be a pointer to the start of an allocated block. S.

(In reply to comment #15) David, I've tested this small suggestion and it appears works fine and resolving bug 505971 issue. I've test this only on Windows 7.

And its working just fine on XP too

(In reply to comment #16) > I think this affects Firefox, in theory, at least, since the code is shared > with Firefox. We very much would like this for the next beta of TB 3, which has > code freeze in about 2 weeks. Someone other than me should probably figure out > if it affects Firefox in actual fact, but I think this fix should go into > whatever versions of gecko Thunderbird ships on, at least. Since I only have > the version of gecko that Thunderbird uses, once this is reviewed, it would be > helpful if someone else could land it on the trunk... Well I can't crash Firefox, probably something special need to do but just surfing pages with SSPI authentication enabled does nothing.

(In reply to comment #8) > In the MSDN sample code Got a link to that? I'm finding it really hard to work out what Unwrap is trying to achieve and reading that is my last resort.

http://msdn.microsoft.com/en-us/library/aa375215%28VS.85%29.aspx describes what DecryptMessage does. In particular, note that it says it decrypts in place, which means that the outbuffer pointer it returns is a pointer into the input buffer, which is also what we see in the debugger. Simon says he could imagine the pointer not being at the start of the buffer, which is why we have the code that copies the output buffer if the pointer is not at the start of the buffer.

Comment on attachment 397446 [details] [diff] [review] fix addressing comments I'm happy that this is theoretically correct, but I don't have a Windows development system handy to test it on.

I've gone to rather great pains to test it, so I'm confident it works. Perhaps Nikolay could be prevailed upon to test this latest patch, instead of the hack I suggested to him earlier...

Works good, no problem so far. Only problem is my builds are with memory leak so can't test if this code leaks anything. Thanks for fix.

Nikolay, I've kicked off a try server build with the patch that you can try, if you want - it shouldn't have any memory leak problems. http://tinderbox.mozilla.org/showbuilds.cgi?tree=ThunderbirdTry However, I'm not really sure you'd be able to detect any leaks if there were any - there are about 10 bytes of memory allocated in this code, per connection to the imap server.

Thanks David, if that just 10 bytes it really difficult to test manually. So it not worth trying. btw Try server actually reports - fail to patch, if it really matters unable to find 'extensions/auth/nsAuthSSPI.cpp' for patching 1 out of 1 hunks FAILED -- saving rejects to file extensions/auth/nsAuthSSPI.cpp.rej extensions\auth\nsAuthSSPI.cpp: The system cannot find the file specified

oh, yeah, it's a moz-central, not comm-central patch. But yeah, if it's just for you to try to see if there's any memory leaks, it's not going to help you.

(In reply to comment #24) > http://msdn.microsoft.com/en-us/library/aa375215%28VS.85%29.aspx describes what > DecryptMessage does. In particular, note that it says it decrypts in place, > which means that the outbuffer pointer it returns is a pointer into the input I don't see anything about inbuffer and outbuffer pointers...

pMessage [in, out] A pointer to a SecBufferDesc structure. On input, the structure references one or more SecBuffer structures. One of these may be of type SECBUFFER_DATA. That buffer contains the encrypted message. The encrypted message is decrypted in place, overwriting the original contents of its buffer. This matches what I see in the debugger as well.

(In reply to comment #33) > pMessage [in, out] > > A pointer to a SecBufferDesc structure. On input, the structure references > one or more SecBuffer structures. One of these may be of type SECBUFFER_DATA. > That buffer contains the encrypted message. The encrypted message is decrypted > in place, overwriting the original contents of its buffer. Right, but what I'm not following is why we pass in two structures, one of type SECBUFFER_DATA, and expect to read the output from the other structure.

If you scroll to the end of this page - http://msdn.microsoft.com/en-us/library/aa380496%28VS.85%29.aspx you'll see that the sample code MS provides does the same thing. We don't actually use the result (the caller throws it away - it just wants to check that it was decrypted successfully - if we try to turn on security levels, then we would not throw the data away, if I understand correctly).

The 1.9.x moz-central branch this needs to go into is freezing in a few days, and we'd really like to get this in before it freezes - let me know if there's anything else Simon or I can do to help explain this (the MS doc is not extremely helpful).

Comment on attachment 397446 [details] [diff] [review] fix addressing comments >+ memmove(*outToken, ib[1].pvBuffer, ib[1].cbBuffer); Thanks, that makes sense now. Nit: use memcpy (or even nsMemory::Clone) here. (I assume a previous revision of the patch memmoved from ib[1] to ib[0].)

OK, thx, I'll attach a new patch in a bit, and request moz-central check-in approval.

carrying forward r/sr, requesting moz-central checkin approval.

dveditz, I'm not clear what all moz-central branches this should land in. I believe it's a bug that goes a long way back. It may not affect Firefox but it definitely affects Thunderbird and SeaMonkey. So I guess I need at minimum to get this into TB 2.0x next and TB 3.0b4...

Needs to land on trunk and 1.9.2 first at least so it get some baking...

Pushed to mozilla-central: http://hg.mozilla.org/mozilla-central/rev/3a1e9ee72015 Leaving this open whilst we bake and get approvals for branch so that it doesn't get lost from the TB 3 blocking list.

Can we get 1.9.2 approval on this? I know it hasn't baked for long, but we are looking to cut Thunderbird 3b4 on Tuesday so it would be good if we could get it in to 1.9.2 before the weekend so we have some hope of getting it into 1.9.1 by Tuesday...

This isn't in a component that any 1.9.2 drivers likely look at. You should ping one or two individually for approval.

pinging dveditz...

dveditz isn't a 1.9.2 driver. See the following page for a list of drivers for various releases. https://wiki.mozilla.org/Releases/Drivers

bsmedberg, ping for 1.9.2 approval. afaik, Firefox doesn't seem to use this code, but Thunderbird needs this fix for our b4 build.

Comment on attachment 399064 [details] [diff] [review] patch addressing Neil's comment a192=beltzner

Checked into the 1.9.2 branch: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/2202e671651d

Comment on attachment 399064 [details] [diff] [review] patch addressing Neil's comment Approved for 1.9.1.4, a=dveditz for release-drivers

Checked into the 1.9.1 branch: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/4aac246ba59f

Can the reporter or someone with a repro environment try this with the nightly 1.9.1 Thunderbird build and see if the issue is actually fixed now? ftp://ftp.mozilla.org/pub/thunderbird/nightly/2009-09-15-03-comm-1.9.1/

It is fixed

Tested on Windows Server 2008 R2 64-bit, Windows 7 32-bit, Window Vista, and Windows XP (all joined to an MS Active Directory). Thanks to all for working on this.

Thanks. Marking as verified for 1.9.1.4.

who does 1.8.1.next approvals? I'd like to get this fix in the next 2.0x TB build...

Sam, Dan, and I do them.

Comment on attachment 399064 [details] [diff] [review] patch addressing Neil's comment Looks like this never landed on the 1.9.0 branch. When you land on the 1.8 branch in CVS please go ahead and land on CVE HEAD as well.

dveditz: thanks for the heads up!

I've landed this on the 1.8 branch but I don't have a CVS head tree pulled right now. I'll try to get one built at some point...

Comment on attachment 399064 [details] [diff] [review] patch addressing Neil's comment removing 1.9.0.18 approval, time to ship. Please re-request approval if this is still wanted by someone maintaining mailnews on the 1.9.0 branch

This is now checked into 1.8.1 for Thunderbird 2.0.0.24. If possible, we need the reporter or someone else with Berkeley e-mail access to verify that it is fixed in TB 2.0.0.24. This can be done using a nightly build from ftp://ftp.mozilla.org/pub/thunderbird/nightly/latest-mozilla1.8/.

I can confirm correct working with SSPI enabled on Win7 Enterprise 32-bit joined to AD and running TB version 2.0.0.24pre (20100211) for both IMAP and SMTP. Thanks for helping to get this addressed for our campus.

Thx, Karl.

Thanks! Marking as verified for 1.8.1.24.