389580 - (CVE-2007-4041) some schemes with %00 launch unexpected handlers on windows

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Daniel Veditz [:dveditz]]

On Windows XP some urls for "web" protocols that contain %00 launch the wrong handler and appear to be able to launch local programs, with limited argument passing. It is not yet clear that this can be used to compromise a machine but we can always fear the worst.

The same behavior is observed using "Run" from the Windows Start menu for the affected protocols (http, https, ftp, gopher, telnet, mailto, news, snews, nttp, possibly others?).

The behavior seems to be that if there's a %00 in the URL for these schemes then the URL Protocol handler is not called, instead the FileType handler is called based on the extension of the full url. The url is then passed to that File handler. For "non-web" URL handlers the URL is passed to the expected handler.

In Firefox browser protocols are handled internally so are not vulnerable, but the mailnews protocols are handed off to the OS and can be abused in this way.

The fix in bug 389106 mitigates the published testcases but doesn't actually "fix" the problem.

http://xs-sniper.com/blog/remote-command-exec-firefox-2005/

Comment 1

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

Attachment: patch

don't load external URLs containing %00

Comment 2

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

Should we do anything about escaped newlines/carriage returns/tabs?

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 273843 [details] [diff] [review]
patch

r=bzbarsky.  I assume that at this point there will never be unescaped nulls in that string, right?

Comment 4

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

correct, nsSimpleURI::SetSpec escapes them (NS_EscapeURI does that even with OnlyNonASCII flag)

Comment 5

[Boris Zbarsky [:bzbarsky]]

I think escaped whitespace should just be passed through, personally... just like escaped spaces, escaped quotes, etc.

Comment 6

[Carsten Book [:Tomcat]]

XP Pro SP2 - IE 7 and full updates/patches with Firefox 2.0.0.5 installed is vulnerable 

XP Pro SP2 - IE 6 and Firefox 2.0.0.5 - not vulnerable 

Tested with a clean new VM.

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 273843 [details] [diff] [review]
patch

sr=dveditz

Seems a shame to hurt any handler legitimately using %00 just because IE7 damaged a subset of common protocols.

On the other hand %00 has caused trouble in the past, maybe on the trunk we should kill it in a more common layer.

Comment 8

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

hrm... I don't really want to kill it in more places than necessary, because it can be used for legitimate reasons too

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 273843 [details] [diff] [review]
patch

approved for 1.8.0.13, 1.8.1.6 and 1.8.1.7.

Remember that 1.8.1.6 is off GECKO181_20070712_RELBRANCH and needs a separate checkin from the 1.8.1.7 on the main 1.8 branch

Comment 10

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

HEAD:
Checking in nsExternalHelperAppService.cpp;
/cvsroot/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp,v  <--  nsExternalHelperAppService.cpp
new revision: 1.323; previous revision: 1.322
done

GECKO181_20070712_RELBRANCH:
Checking in nsExternalHelperAppService.cpp;
/cvsroot/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp,v  <--  nsExternalHelperAppService.cpp
new revision: 1.290.4.12.6.3; previous revision: 1.290.4.12.6.2
done

MOZILLA_1_8_BRANCH:
Checking in nsExternalHelperAppService.cpp;
/cvsroot/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp,v  <--  nsExternalHelperAppService.cpp
new revision: 1.290.4.15; previous revision: 1.290.4.14
done

MOZILLA_1_8_0_BRANCH:
Checking in nsExternalHelperAppService.cpp;
/cvsroot/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp,v  <--  nsExternalHelperAppService.cpp
new revision: 1.290.4.2.4.4; previous revision: 1.290.4.2.4.3
done

Comment 11

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

I backed this out of trunk for the moment, the tree was red

Comment 12

[juan becerra [:juanb]]

Please help us with tests cases to verify this bug. Not being able to reproduce the problem explained in the blog may serve as verification, but if there's anything else we could test that you can think of, please make suggestions.

Comment 13

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

Checking in nsExternalHelperAppService.cpp;
/cvsroot/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp,v  <--  nsExternalHelperAppService.cpp
new revision: 1.326; previous revision: 1.325
done

Comment 14

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

As for testcases, make sure that URLs containing null bytes don't work, e.g.:

<a href="mailto:[e-mail]?subject=foo%00bar">Link</a>
<a href="mailto:[e-mail]?subject=foo&#x00;bar">Link</a>

Perhaps also a JavaScript-created link with \x00 in the href.

Comment 15

[u279076]

Attachment: TESTCASE: Based on code in comment #14

I have tested the 2 mailto links to not launch the mail client.
The j/s link does however.  Not sure if I coded this right as I am not adept at j/s.

Comment 16

[u279076]

Attachment: TESTCASE: Revised above

Comment 17

[Juergen Schmidt]

Secunia has done some more research on this issue. They claim, that not "%00" but "%" itself triggers the problem. In fact the modified link

<a href="mailto:%../../../../../../windows/system32/cmd".exe ../../../../../../../../windows/system32/calc.exe " - " blah.bat>Mailto:%</a>

starts calc.exe on my Windows XP system. So please check, that your patch includes this.

bye, ju

Comment 18

[Juergen Schmidt]

Attachment: Testcase for Secunia mailto:%

Secunia detected, that "mailto:%" is enough to trigger the bug.

Comment 19

[Mike Schroepfer]

It appears that IE7 changes *both* the behavior of windows and its own encoding of the URLs.  We could use some help trying to understand the behavior of Windows and IE with the different scenarios here.  In particular in bug 389106 there is a nice list of protocol handlers for windows and an exe that you can register to get the URL output.  The tests to run:

For each protocol register in that list register the .exe as the handler and try URLs's with % and %00 in each of the following scenarios:

System: XPsp2IE6:

Run from Start Menu -> resulting URL passed to handler
Run from IE6 -> resulting URL passed to handler
Run for FF2006 -> resulting URL passed to handler

System XPsp2IE7

Run from Start Menu -> resulting URL passed to handler
Run from IE7 -> resulting URL passed to handler
Run for FF2006 -> resulting URL passed to handler

Comment 20

[Juergen Schmidt]

Attachment: % triggers the bug

added Secunias original demo without detour over cmd.exe.

Comment 21

[Robert Strong (they/them - no direct email)]

Attachment: installer that sets up all of the protocol handlers

This will add the windows protocol handlers under HKEY_CURRENT_USER pointing to the testurl.exe which is also contained within this installer. After you have finished it will attempt to restore any protocol handlers it replaced and it will just delete protocol handler it has added. I say attempt because I found that it wasn't reliable but I found that displaying a messagebox in the macro appears to have fixed this. If it doesn't work the original regkey has been saved to the installation directory's backup directory and can manually delete the key under HKEY_CURRENT_USER\Software\Classes\ and add it back by double clicking the protocol's file (e.g. protocolname.reg).

The default install location is in your documents folder and to uninstall you will have to run uninstall.exe from within this folder (the default name is protocoltest).

Comment 22

[Robert Strong (they/them - no direct email)]

Also, if testurl.exe doesn't launch you may have the associated app for a protocol running. By using the HKEY_CURRENT_USER registry keys - which is safer in regards to restoring the settings - I am unable to overwrite the ddeexec keys so the app uses them they will pick up the url if it is already running. All you should have to do to remedy this is exit that app. I also didn't add keys for the shell protocol so don't bother testing it. So far my tests show that the keys are restored successfully with the additional messagebox.

Comment 23

[Juergen Schmidt]

Attachment: a couple of testcases using % and not containing any %00

sorry - had the encoding wrong in the last testcase.

Comment 24

[Robert Strong (they/them - no direct email)]

One last note... hopefully... if the app does support dde the OS will display an error even though testurl.exe launched successfully. The error can be ignored and the test output is just as valid.

Comment 25

[Robert Strong (they/them - no direct email)]

Attachment: NSIS script

Besides NSIS this requires the testurl.exe to be in the same dir as the script when compiling and the registry plugin from http://nsis.sourceforge.net/Registry_plug-in

Comment 26

[Daniel Veditz [:dveditz]]

(In reply to comment #17)
> They claim, that not "%00" but "%" itself triggers the problem.

It's not mere presence of '%', a valid percent-encoded value (other than %00) does NOT trigger the problem (e.g. %20, %FF). % followed by two characters that are not valid hex digits, or %00 trigger the problem.

> So please check, that your patch includes this.

Our patch does not. But IE7 will pass an unencoded '%' to most protocols (except the "web" ones) so we can't strip it willy-nilly and still be compatible. We're in kind of a bind here. Systems with IE6 installed are different, too, as is Vista.

Leaving apart the system-specific aspects we could do something like

 webprotocols = /^https?:|^ftp:|^mailto:|^s?news:|^nntp:|^telnet:|^gopher:/;
 badencoding = /(^[^?#]*)%(.?[^0-9a-fA-F]|00|$)/;
 if (webprotocols.test(uri) && uri.match(badencoding)) {
    throw "bad encoding in schemes treated special by Windows";
 }

Except we'd have to do that in C++ in the windows-specific LoadURIInternal()

Comment 27

[Daniel Veditz [:dveditz]]

... and first verify that the list of "web" protocols is complete and that our conclusion that only %00 and %-invalid trigger the problem is correct.

Comment 28

[Daniel Veditz [:dveditz]]

Attachment: testpage for the protocol list

Here's a testpage built from the protocol list in attachment 273656 [details] (bug 389106)

Comment 29

[Daniel Veditz [:dveditz]]

Using an unreadable/unmaintainable regexp is asking for trouble. Maybe we should just give up and bypass ShellExecute() entirely, pulling the command-line out of the registry and call it ourselves.

But then we have to reverse-engineer what replacement parameters ShellExecute() will handle. In the list Rob put together in bug 389106 I can see "%L" and "%l" (upper- and lower-case L) mixed in with the usual "%1" (digit 1), as well as a couple with %1 %2 (?). Is it just the first %x?

The docs for registering a protocol handler on windows say to use "%1"

Comment 30

[Jim Mathies [:jimm]]

Based on the email discussions going on, it looks pretty obvious we need to rework this handling completely. I'll try and get things kicked off here - 

In general, with IE6 installed we appears immune to the mailto example, however this isn't really important. As someone pointed out via email, we don't handle these web protocol handlers correctly. We generally trust SE to keep us safe, and that doesn't appear to have been the right approach.

The current proposal is to do something along these lines with some comments added -

1. Detect that an external protocol handler is invoked (mailto:blah)

2. Parse out the protocol definition, i.e. everything to the left of the first colon (mailto)

3. Look up the protocol definition in the registry under HKCR 

4. If there is no value under the protocol definition called “URL Protocol” this is not a valid protocol. Error out.

5. If it is a valid protocol open HKCR\<protocol>\shell\open\command and read the default value 

This is where things get a little sticky, there are multiple formats here we would need to deal with, and we may want to expand environment strings at this point as well.

 - read the full command template from shell/open/command
 - detect "known" executable types we want to support (an application, a rundll32.exe handler, others) 

There are other ways in which these handlers can be configured besides an app or rundll, a good example might be rlogin on vista -

url.dll,TelnetProtocolHandler %l

(To mitigate this quickly we might adopt a short list of supported handler templates, and expand on that as bugs related to the more exotic handlers show up.)
 
 - separate the handler from the parameter template
 - replace the appropriate parameter token with our url  (%1, %l %L - we need to track down the docs on what the differences are here, honestly ive not run into '%L' until today.) 

Also should we be applying any security related sanity checks on the URL were about to hand off?

6. Use shellexecute to launch the handler, with the appropriate parameter string.

Just a start, more research is needed to make sure we do not open up a new hole by handling these inappropriately.

Comment 31

[Jim Mathies [:jimm]]

Also, some test results - last night I spent mostly with IE6, and was unable to get calc to launch. After an upgrade to IE7, I've confirmed the calc / mailto thing works. The parameter looks like this:

There are 1 arguments in the URL
Argument 0:     mailto:test%../../../../testurl.exe.cmd

Comment 32

[Jim Mathies [:jimm]]

Some additional notes - 

- %l or %L vs. %1 implies the use of a long file name
- a dll handler without rundll in the template implies the use of rundll32.exe.

Comment 33

[Harry Johnston]

I haven't found any way to exploit this without having an unencoded double quote in the URL; so perhaps the changes in bug 389106 would also protect us from this?

Comment 34

[Daniel Veditz [:dveditz]]

Although I haven't run across any that actually use these yet, %d and %0 also replaced with the URL for registered handlers. Other %-digits are replaced with nothing, most other %-letter are replaced with the letter except:

%h is replaced with 0
%i is :#:PID  where PID is the process ID of the calling program
              (IE or Firefox) and the number (#) changes every call
              but I'm not sure what it is. Handle? Clock slice?
%s is replaced with 1

Multiple replacements happen, too, so at least we don't have to look for one or the other. "%1" "%L" "d" results in 3 copies of the URL passed to the handler.

Comment 35

[Daniel Veditz [:dveditz]]

And then we'd have to duplicate the DDE calling path, too.

#4 in comment 30 is bug regardless of whether we use ShellExecute as we are now or do the more indepth duplication of Windows behavior. bug 389611, fixed on trunk, ready to land on branch if we want to re-spin for it.

Comment 36

[Daniel Veditz [:dveditz]]

We received a response from Microsoft that in addition to the schemes listed in comment 0 they also treat "wais", "file", and "mk" in this manner when IE7 is installed. The normalization is done by the CreateUri function in accord with RFC 3986 as described at http://msdn2.microsoft.com/en-us/library/ms775098.aspx

Comment 37

[Jim Mathies [:jimm]]

Attachment: createuri experimental patch

This is purely an experimental patch that avoids major changes by vetting URI through the newer CreateUri available post IE7.

Comment 38

[Stephen Donner [:stephend] Not actively reading bugmail]

I ran Rob's NSIS installer script to register some protocols listed on
https://bugzilla.mozilla.org/attachment.cgi?id=274255, as well as the
testurl.exe command-line shell, to verify that the first URI in the bugzilla
attachment triggers the testurl.exe to run, but not the 2nd and 3rd ones, which
usually have "%00" or "%[invalid char]".  Verifying this as fixed, then, using
a Thunderbird version 1.5.0.13 (20070809) (release candidate) build.

Replacing fixed1.8.0.13 with verified1.8.0.13.

Comment 39

[Daniel Veditz [:dveditz]]

Created bug 394974 to track the remaining work (namely, addressing comment 17 on including the work done in comment 37)

Comment 40

[Stephen Donner [:stephend] Not actively reading bugmail]

I've actually done the verification legwork for both this and bug 394974 over in that bug (I did more than I mentioned there, actually; I ran the first of the set of three as well, but only reported on the others).

See https://bugzilla.mozilla.org/show_bug.cgi?id=394974#c27 for more details.

Additionally, I ran Juergen Schmidt's testcases in this bug using Windows XP SP2 with IE 6 installed, Windows XP SP2 with IE 7, and Vista.

Replacing fixed1.8.1.8 keyword with verified1.8.1.8 keyword, since I used Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.8) Gecko/20071008 Firefox/2.0.0.8 on the three OS flavors to test.