Make OpenLibary() and Import safe from dll-hijacking.

[Rescator]

SetDllDirectory()
http://msdn.microsoft.com/en-us/library ... 85%29.aspx
http://blogs.msdn.com/b/david_leblanc/a ... tacks.aspx

Solution for older OS http://blogs.msdn.com/b/david_leblanc/a ... tacks.aspx

Fred, any chance you could make PureBasic always do a SetDLLDirectory("") at the very start of the program (before any dynamic dll loading takes place).
SetDLLDirectory() is available from Windows XP with SP1 and Server 2003.

SetDLLDirectory("") tells the OS that it should remove the current directory from the dll search path,
this twarts the majority of dll-hijacking/dll-preloading exploits.
The regular search paths are untouched, so the OS will still look in the exe's directory for the dll and then the system folders etc.

For XP "Gold", W2K and older the alternative technique shown in the second blog post is needed.

If these changes are done for the next PureBasic release then PureBasic would be "safe" "out-of-the-box" against the dll-hijacking/dll-preloading attacks.

Obviously there may still be an issue with load-time dll's (aka Import feature) as a Vista dll may not exist on XP and thus such a dll search could still be exploited.
dwmapi.dll is such a dll, I'm not sure how that can be fixed, is there a way to force Import (load-time dlls) to NOT search current directory?
Maybe one of those confusing MSDN pages about manifests has a solution for that?
or is it possible to use SetDLLDirectory("") before the Import *happens* at run time? I believe the kernel actually uses LoadLibary for load-time dlls but it's before PureBasic can do a SetDLLDirectory("") I suspect!
Ideally SetDLLDirectory("") should be done right after kernel32.dll is loaded (where SetDLLDirectory is found) but before any other dlls are loaded.

Anyway, SetDLLDirectory("") and the fix in the second blog post should fix half the issue related to LoadLibary and other related APIs being vulnerable as current directory will not be searched any longer.
And if anyone has an issue with this then make it the default but allow developers to disable it (thus assuming they know what they are doing).

Personally I can't think of ever needing to load a dll from the current directory, the exe directory is the usual place, and if it's a sub folder in the exe folder then I explicitly load them using the full path anyway so...


Oh and for those wishing to individually secure their system across the board there is a "fix" from Microsoft http://support.microsoft.com/kb/2264107 for XP and later.
But obviously we developers can't demand that users apply this fix or expect all of them to be aware of it so the request above is still valid regardless.


Oh and for those that do not know how this exploit works, it's laughably simple (and published all over the web by now).
Trick someone to open what appears to be a benign document of some sort, in fact it could even be a real and clean document, but in the same folder as the document is a dll.
For example if it was a XP system it could be a normal file but which is used by a program that works on both XP and Vista+ (and makes use of dwmapi.dll if available like Firefox might do).
As soon as the program starts it will look for the dll, and it find it in the current directory (where the document is) and load the dll, the actual document is just a red-herring,
the dll that is now loaded can do whatever evil thing it wants to do.

This exploit would never have been possible if Microsoft had not allowed dll search in the current directory. (it's a behavior holdover from the old DOS days actually)

http://www.reuters.com/article/idUS2168761020100825
Acros discovered at least 121 exploits in 41 Microsoft applications, and if you look around on the net you'll see that potentially thousands of other non-microsoft applications are vulnerable as well.
(PS! Make sure that your browser and utorrent and whatever else is updated as most of the more popular apps out there have/has/are fixing the issue. (using SetDLLDirectory("") and the alternative fix for older OS' most likely.)
Here is the report and proof-of-concept on uTorrent's vulnerability http://www.exploit-db.com/exploits/14726/

Creepy how easy it is right?
Basically you could be exploited by simply opening a html file using a older Firefox on XP if an attacker/site manages to sneak in a dwmapi.dll along with with the html, and I doubt many people would think twice about a dll just sitting there along with a html for example. We'll I'll certainly do from now on but...)

Anyway, now that I've given you all a nightmare to dream about I'll continue looking through all my apps and make sure that I use the two solutions mentioned above and make sure to use full paths to any plugin dlls my apps may use etc.

Oh and to depress you all further, here's a "small" list of some apps that was/are vulnerable http://www.corelan.be:8800/index.php/20 ... cial-list/

EDIT: Changed the topic to reflect the (now) thread subject better.

[Rescator]

Did some more digging on making load-time dlls more secure from the exploit as well.
It seems like the solution to the Import dlls issue is to use Delayed DLL loading and use SetDLLDirectory("") before any dll's functions are referenced obviously.
Anything older than XP SP1 and Server2003 (like XP gold) will still be vulnerable since SetDLLDirectory() is not available though.

[Rescator]

Oh and one more thing that I feel should be mentioned, although it has nothing to do with this topic exactly.

The same current directory issue exists with EXE's and it's even much worse there as there exists no SetEXEDirectory("") API at all,
so if your app just uses RunProgram("blah.exe") and you haven't made sure that the current directory for your program is actually set to a path you trust (or that you specify)
you could unintentionally be executing a hostile exe that is in the same folder as the document that caused your app to launch.

Here's a good link explaining simply what the whole issue is with dll's and exe's and current directory and why it's up to us developers and not MicroSoft to "plug" this hole (sadly):
http://www.antionline.com/showthread.php?t=279098

This Microsoft MSDN article is a must read: http://msdn.microsoft.com/en-us/library ... 85%29.aspx
Also note the comment that using SetEnvironmentVariable(TEXT("PATH"),NULL); might also be worth considering.

[Rescator]

Hey Fred, I had a thought. Is it possible to make a "dummy" dll (lib) of sorts that could be placed after the kernel32.dll import so that the windows dll loader will "load" it and execute the entry point (which would simply do a SetDllDirectory("") if available in kernel32.dll obviously), and then the following dll's will thus load (through LoadLibary via the kernel like normally) not use the current directory.
Or am I mistaking in the way the kernel loads dlls/libs and their order.

I'm assuming that the kernel loads each import'ed lib in order, executing the entry point and possibly the attachprocess etc, and then it moves on to the next import'ed lib and does the same?
So just "sneaking" in SetDllDirectory("") inbetween kernel32.lib and whatever is next would solve it without having to use /delayload in POLINK and importing the delayimp.lib etc. and the some of the restrictions delayload has (like not being able to import data, or so MSDN says at least)

Just to re-iterate, would the following be possible?

1. import/load kernel32.dll
2. import/load "dummy" lib
2a. point the entry point of the dummy lib to code that does the following:
2b. check if SetDllDirectory() exists in kernel32.dll if it does then...
2c. call SetDllDirectory("")
2d. return true, so the loading can continue...
3. import/load the rest of the libs like normally (but a lot safer on XPSP1/W2K3 and later from now on obviously).

[Thorium]

The PE loader first mapps ntdll.dll to the process memory then kernel32.dll then the .exe and after that it processes the imports of the .exe in the order they are stored in the import table.

Actualy the exploit could be used by just name the dll kernel32.dll and wrap all the functions to the real kernel32.dll which can be loaded with ntdll.dll.

[Fred]

While all this subject is interesting, i really think it's not our job to try to fix a MS flaw...

[Trond]

SetDLLDirectory() is available from Windows XP with SP1 and Server 2003.

Making this function basically useless, as the real issue is with XP pre-SP1 and older. Dlls loaded with Import aren't loaded from the current directory anyways from SP1 and on. Dlls loaded with LoadLibrary() are controlled by the programmer so there isn't really a problem.

[Fred]

So if imported lib aren't affected with this, and the only thing to add to a PB exe is a dynamic call to 'SetDLLDirectory("")', it sounds like a reasonable request (basically, you can do it yourself at every program start, right ?)

[Trond]

So if imported lib aren't affected with this, and the only thing to add to a PB exe is a dynamic call to 'SetDLLDirectory("")', it sounds like a reasonable request (basically, you can do it yourself at every program start, right ?)

Imported lib/dll's are affected when the program is ran on an os before XP SP1. Since this function is only available after XP SP1 it doesn't help then. And after SP1 it's not needed for import.

When it comes to LoadLibrary, it's just as easy to change the current directory into something sensible before doing calling LoadLibrary().
SetCurrentDirectory(GetPathPArt(ProgramFilename())) should work just as well as SetDllDirectory().

In fact better, because SetDllDirectory() has a design flaw in itself: It you pass a pointer to an empty string the current directory is removed from the search path. But, if you pass a null pointer, the current directory is added to search path again.

So if Fred adds a SetDllDirectory() in the startup code it will be no use if an overcautious user add it again just before loadlibrary, but inadvertently passes a null pointer, like this: SetDllDirectory_(@PresumedEmptyStringButReallyANullPointer.s)

The only way that's really safe and fool-proof for Import is to run the program on XP SP1 and newer.
The only way that's really safe and fool-proof for LoadLibrary() is to set the current directory to a known good value before using it.

[freak]

> The only way that's really safe and fool-proof for LoadLibrary() is to set the current directory to a known good value before using it.

Changing the current directory behind the scene is not a good idea. If anything, the programmer should do that explicitly.

IMHO, the SetDLLDirectory("") should only be done for the OpenLibrary() PB command (maybe on each call?). If people want to go the API way, they have to think about these things themselves, and as Trond sais the user can just as easily make it unsafe again if they don't know what they are doing.

Oh, and if somebody is still using pre XP SP1, they should not be surprised if they have security problems anyway.

[Trond]

> The only way that's really safe and fool-proof for LoadLibrary() is to set the current directory to a known good value before using it.

Changing the current directory behind the scene is not a good idea.

I agree with that. The programmer must do it.

[Rescator]

While all this subject is interesting, i really think it's not our job to try to fix a MS flaw...

Yeah I know, and I wish too that MS would just rip out current directory from the loading search path,
but too much old software relies on that behaviour, only thing I can think of are plugins so I don't understand what software needs current dir loading,
but MS has stated they do not intend to fix this as it would break too many apps. (and as many has noticed this "hole" has been her for a decade now I think?)

SetDLLDirectory() is available from Windows XP with SP1 and Server 2003.

Making this function basically useless, as the real issue is with XP pre-SP1 and older. Dlls loaded with Import aren't loaded from the current directory anyways from SP1 and on. Dlls loaded with LoadLibrary() are controlled by the programmer so there isn't really a problem.

Are you sure about that? Article (pref. a MS one or some expert site/blog) or search terms that works (on google) because import is a PureBasic ter, so I haven't read any articles on that yet

Because if this is true and that the kernel (ntdll.dll right?) ignores current dir etc. from SP1 onwards for implicitly loaded dll's then I can relax more easily,
as the only concern then would be to make sure I'm aware of where current directory is pointing and maybe use SetDllDirectory("") if available before I explicitly load dlls.

So if Fred adds a SetDllDirectory() in the startup code it will be no use if an overcautious user add it again just before loadlibrary, but inadvertently passes a null pointer, like this: SetDllDirectory_(@PresumedEmptyStringButReallyANullPointer.s)

Well, I guess the debugger could show a warning that SetDllDirectory("") is set automatically on a OS that supports it (or just mention it in OpenLibary manual page) if someone knows about SetDllDirectory() then I hope they know what they are doing.
Besides, there is also a matching GetDllDirectory() so you can check what it's set to.

> The only way that's really safe and fool-proof for LoadLibrary() is to set the current directory to a known good value before using it.

Changing the current directory behind the scene is not a good idea. If anything, the programmer should do that explicitly.

Yep! Which it what SetDllDirectory() allows you to do, "" removes it, #Null restores normal behaviour, and "some path to some folder that becomes the Dll load directory" are the three ways to use this API, shame there isn't a SetExeDirectory(), then again RunProgram() does support setting a working directory (which basically is the same as current directory) and I know of no program that runs a exe during it's startup anyway (unlike with dlls).

IMHO, the SetDLLDirectory("") should only be done for the OpenLibrary() PB command (maybe on each call?). If people want to go the API way, they have to think about these things themselves, and as Trond sais the user can just as easily make it unsafe again if they don't know what they are doing.

SetDllDirectory() as far as I read from MSDN only needs to be used minimally once per process as it's process wide (so calling it from multiple threads in a program will have the same uses as changing the current directory from multiple threads, I believe this had even been a bug issue in PureBasic a some time in the past)


Anyway, what my I want to know is, if Import is truly safe from current directory exploit/dll hijacking on XP SP1 / 2K3 and later (links please Trond
And if it is then maybe Fred or Freak are kind enough to look at what can be done to make it easier to make OpenLibrary() safer for people (if people use LoadLibrary then they are doing direct API stuff anyway), either a tweaked OpenLibrary() that checks if SetDllDirectory() has already been done or not, or if it's done once at program start maybe.

It would be nice to know that PureBasic "out-of-the-box" would not be vulnerable to the dll hijacking,
experts could disable (IDE option, compiler directive?) if they like to control the behavior themselves instead etc.

Of course current directory is not the only issue, as SetEnvironmentVariable("PATH",#Null) might be needed to make sure nothing lurks there either. But usually this means an attacker must have had the ability to mess with the PATH var which is rare as that would mean some form of execution ability anyway, and as most know if you can execute on a system there is no need to exploit anything anyway since they have control.

My main concern was (and still is) issues like somebody making say a program that uses Import to load dwmapi.lib on Vista+ this is ok as the kernel knows that dwmapi.dll is a "special" dll and will ALWAYS check system32 first etc, so current directory is never even looked in on Vista+ for that lib.
But on W2K3 or XP or older dwmapi.dll does not exist (even on XP SP3) so instead of the program (or OS rather) complaining that the dll is missing it will load it and the entry (attachprocess) code in the dll is allowed to execute.
But if as Trond says that XP SP1 and later is safe from Import being exploited that way then I agree that there isn't that much need to worry about it (most XP users have SP1 at least right?)

OpenLibrary() is still an issue though, heck, what about a flag to turn off/on current directory and PATH searching?

In any case I'm slowly going through all my code and making sure that I only give FULL paths to OpenLibrary() (and same for LoadLibrary(), RunProgram() etc.)
I'm also pondering using full paths even when I wish to load system dlls.

I plan on using GetSystemDirectory() (win9x ?) or alternatively SHGetFolderPath() (W2K/XP +) or SHGetKnownFolderPath() (Vista+)
and get the system32 directory and get the path directly so I can do c:\windows\system32\kernel32.dll (just as an example) when using OpenLibrary()

Here is the info on the dll path search order: http://msdn.microsoft.com/en-us/library ... 85%29.aspx
If you look at that you'll see most is really old legacy paths.
And normally you would only ever need to load a dll from either:
"The directory from which the application loaded."
or
"The system directory" (in a few rare cases maybe "The Windows directory." but on my Win7 install I only see twain.dll in there (scanner/cam related lib) but most dlls live in system dir.)

If I could ignore "The 16-bit system directory", "The current directory.", "The directories that are listed in the PATH environment variable" then I'd happily do so,
heck I'd be fine with always full paths even, I can always get the current dir or the system dir myself and string together the full path to those if I really need to.
I'd rather have my program/code annoy me to hell and back that a dll is not found due to me messing up the full path than leaving me open to the current dir issue.

I.e: I could always make a OpenLibrary() wrapper that gets the system path, the windows path and the exe path and attempt to OpenLibrary() the full paths to them and if not found then fail right there.
Obviously I'd rather have Fred or Freak build in that safety behaviour into PureBasic so I don't have to remember to use such a wrapper all the time but...

But I can do nothing about Import as that is in the linker / PE loader stage etc. So please Trond, dig up the source about intrinsic loaded dlls being safe from current dir on XP SP1+

[Rescator]

Um Trond, I did some tests here. It seems you "may" be wrong. Check this out:
http://www.mediafire.com/?9q9pst8ncwreol3 (source included in case you don't trust the binaries and would like to compile yourself).

Here's how to do this test.
Inside the test folder there is a dtest.exe all this does is use Import to load dreal.dll
Inside the test\fake\ folder is a dreal.dll (this is actually the dfake.pb code, dll was simply renamed)

Now to test this do the following.
Double click the test.bat (the only thing this does is execute "..\dtest.exe" in the parent dir.

You should see the messagerequester "NotEvil?" "Nope! I'm actually the real thing!"

Now rename the dreal.dll that is together with dtest.exe just name it dreal_.dll or something.

Run the bat again, if the system is vulnerable to dll hijacking,
then you should see the messagerequester "NotEvil?" "Yes I am! *evil laugh*"

Now the test.bat is just to test this quickly. (oh and use Process Monitor from sysinternals/winternals to see what actually happens if you wish)
But it could just as easily be "myplaylist.pls" if your program is a media player for example.
What actually causes the exe to launch doesn't matter at all, as long as the current directory (where the file or whatever was clicked/started from) also has a dll along side it.
dlls like kernel32.dll are safe from this (at least from XP SP1, possibly even earlier)

But not if the app assumes that dwmapi.dll (the new fancy Vista+ look stuff is in there if I recall correctly) is available and uses Import.
Obviously this is bad as you really should dynamically load features instead but, the issue is that we as "end" programmers can not do any workarounds at all in regards to Import or ImportC.

A lot of devs these days are fixing more and more of the loadlibrary "hole" these days, but I'm not sure what they do about import'like loading.

I mean, it would be a nice bullet point in PureBasic's featurelist regarding being more secure than other languages in regards to dll hijacking at least, heck over 41 MS apps (many of which are still in/for Win7 has the dll hijack issue still unfixed).

So if anything can be done to make Import'ing safer that would be awesome.

And before anyone say that "dwmapi.dll is for Vista+ only? Why not just check the OS and refuse to run?"
erm. because all Import'ed dlls has already initialized by that point anyway, so a dll hijack may just have taken control of your process by this point anyway.

PS! The NotEvil() test function in the lib was just to quickly do a test. The evil dll could just as well had a NOP for that and done it's dirty deeds in AttachProcess() instead.

[Trond]

I tested it before writing that Import is not affected, but I may have done something wrong during testing. I will have a look again.

Edit: I didn't bother to recompile (I'm not as paranoid as you), but both dtest.exe and test.bat shows "I'm the real thing". If you rename the REAL dtest.dll to something else, then obviously it shows the false message. This is not a security issue, as you would need access to the program's files to rename the dll, and if you have access to the program's files, then you already have full control.

Source for "old" (vulnerable) dll search order and "new" (safe) search order:
http://msdn.microsoft.com/en-us/library ... S.85).aspx

If SafeDllSearchMode is enabled, the search order is as follows:
The directory from which the application loaded.
The system directory. Use the GetSystemDirectory function to get the path of this directory.
The 16-bit system directory. There is no function that obtains the path of this directory, but it is searched.
The Windows directory. Use the GetWindowsDirectory function to get the path of this directory.
The current directory.
The directories that are listed in the PATH environment variable. Note that this does not include the per-application path specified by the App Paths registry key. The App Paths key is not used when computing the DLL search path.

If SafeDllSearchMode is disabled, the search order is as follows:
The directory from which the application loaded.
The current directory.
The system directory. Use the GetSystemDirectory function to get the path of this directory.
The 16-bit system directory. There is no function that obtains the path of this directory, but it is searched.
The Windows directory. Use the GetWindowsDirectory function to get the path of this directory.
The directories that are listed in the PATH environment variable. Note that this does not include the per-application path specified by the App Paths registry key. The App Paths key is not used when computing the DLL search path.

As you can see, the flaw comes from searching the current directory before the system directory. Importing dlls that are vista-only is not a security issue, as the program won't run at all on XP by default. Thus it is unlikely to be installed by the computer owner. If anyone else can install it, then this means they can install anything, so he already has full access, so being able to load a dll from a different directory than normal isn't any further risk when you're already running a foreign exe.

[Rescator]

As you can see, the flaw comes from searching the current directory before the system directory. Importing dlls that are vista-only is not a security issue, as the program won't run at all on XP by default. Thus it is unlikely to be installed by the computer owner. If anyone else can install it, then this means they can install anything, so he already has full access, so being able to load a dll from a different directory than normal isn't any further risk when you're already running a foreign exe.

yep, current directory is the major issue here (and partially PATH).

so being able to load a dll from a different directory than normal isn't any further risk when you're already running a foreign exe.

But that's the thing. it could be a .pls or .txt or .xml or .icon or whatever, they just need to get the dll on the system together with whatever innocent file it might be.

This could be something as benign as opening a zip archive with 7zip, and double clicking say a .png file,
7zip temporarily extracts files (if it's smart it extracts just the .png obviously) but the file ends up in a temp folder, now if a evil dll ends up in the same place,
well if the dll the program looks for is not found elsewhere first it will find it in the current dir (where the png is)
Does not need to be a temp folder, it could be a USB stick with an image (and a dll in the same folder), click the image and you could be unlucky if your fav software is vulnerable.

So even an experienced user may fall for the trap.

Which was the point of this whole thread in the first place, if Fred is able to utilize SetDllDirectory("") in the init phase somehow, maybe by simply making a tiny dummy lib that calls it but is placed right after kernel.lib is imported and before any others (SetDllDirectory("") would be done during the AttachProcess() stage of the dummy lib).

I was wondering if that is possible. Then when all the import stuff is done with and the program starts proper SetDlllDirectory(#Null) could be done to restore default behavior.
That would take care of the import hole. (but if it requires delay loading all import libs then that is probably too much work)

Now when it comes to OpenLibrary() that could be fixed with some minor code maybe and optional flag to enable/disable extra safety. (or just a compiler directive at the start of the prog or something, #PB_Compiler_SafeOpenLibrary *shrug*)
Or if that is too much work to do, a friendly reminder in the manual under OpenLibrary when on windows. *double shrug*

As I said, I can handle the OpenLibrary stuff easily myself, but when it comes to the Import/ImportC part I can't do a thing about it, nor do I know exactly what happens during that stage of a program, or even if it's possible to sneak in SetDllDirectory("") in there someplace.