Still if I could be as loved as Mike Myers it might be worth using them myself. Wait... |
path = GetExecutablePath(); if(ValidAuthenticode(path)) { cmdline = '"' + path + '"'; CreateProcess(NULL, cmdline, ...); }
The security issue appears when the executable path is influenced by untrusted code. The ValidateAuthenticode function verifies the file is signed by a specific certificate. Only if that passes will the executable be started. This hits so many bugs classes as it is, poor input validation, TOCTOU between the validation and process creation and also failing to pass the first argument to CreateProcess. But the sad thing is you see it in real software by real companies, even Microsoft.
Now to be fair the one thing the code does right is it ensures the path is double-quoted before passing it to CreateProcess. At least you won't get some hack hassling you for executing C:\Program.exe again. But process command lines and file paths are two completely different interpretation domains. CreateProcess pretty much follows how the standard C Runtime parses the process path. The CRT is open-source, so we can take a look at how the executable name is parsed out. In there you'll find the following comment (the file's stdargv.c if you're curious):
/* A quoted program name is handled here. The handling is much simpler than for other arguments. Basically, whatever lies between the leading double-quote and next one, or a terminal null character is simply accepted. Fancier handling is not required because the program name must be a legal NTFS/HPFS file name. Note that the double-quote characters are not copied, nor do they contribute to numchars. */You've got to love how much MS still cares about OS/2. Except this is of course rubbish, the program name doesn't have to be a legal NTFS/HPFS file name in any way. Especially for CreateProcess. The rationale for ignoring illegal program names is because NTFS, like many file systems have a specific set of valid characters.
What's a valid NTFS file name you might ask? You can go look it up in MSDN, instead I put together a quick test case to find out.
#include <stdio.h> #include <Windows.h> #include <string> int wmain(int argc, WCHAR* argv[]) { for (int i = 1; i < 65536; ++i) { std::wstring name = L".\a"; name += (WCHAR)i; name += L"a"; HANDLE hFile = CreateFile(name.c_str(), GENERIC_READ | GENERIC_WRITE, FILE_SHARE_DELETE, NULL, CREATE_ALWAYS, FILE_FLAG_DELETE_ON_CLOSE, NULL); if (hFile == INVALID_HANDLE_VALUE) { printf("Illegal Char: %d\n", i); } else { CloseHandle(hFile); } } return 0; }And it pretty much confirms MSDN, you can't use characters 1 through 31 (0 is implied), 32 (space) has some oddities and you can't use anything in <, >, :, \, ", \, /, |, ?, *. Notice the double-quote sitting there proud, in the middle.
Okay, let's get back to the point, what's this got to do with ADS? If you read this you'll find the following statement: "Any characters that are legal for a file name are also legal for the stream name, including spaces". If you read that and thought it meant that stream names have the same restrictions as file names in NTFS I have a surprise for you. Change the test case so instead of '.\a' we use '.\a:' we find that the only banned characters are, \, / and : quite a surprise. For our "bad" code we can now complete the circle of exploitation. You can pass a file name such as c:\abc\xyz:file" and the verification code will verify c:\abc\xyz:file" but actually execute c:\abc\xyz:file (subtle I know). And the crazy thing about this is there isn't even a way to escape it.
The moral of the story is this, you can never blindly assume that even a single interpretation domain works how you expect it to do, so when you mix domains together pain will likely ensue. This is also why Windows command line processing is so broken. At least *nix passes command line arguments separately, well unless you use something like system(3) (and for that you'll be punished). Making assumptions on the "validity" of a file path just seems inherently untrustworthy.