Page 2 of 2

Re: Use SetDllDirectory() by default to prevent dll-hijackin

Posted: Thu Sep 16, 2010 6:41 pm
by Trond
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.
No, it can't. Because the program will only load the malicious dll if the real one is missing or renamed. And if malicious code can delete and rename your programs you have bigger security problems.

Re: Use SetDllDirectory() by default to prevent dll-hijackin

Posted: Thu Sep 16, 2010 6:48 pm
by Rescator
Trond wrote:Importing dlls that are vista-only is not a security issue, as the program won't run at all on XP by default.
Wrong.
If it's missing then yeah the OS will react with: "dtest.exe - System Error" "The program can't start because dreal.dll is missing from your computer. Try reinstalling the program to fix this problem."

but if a malign dll of the same name is in the current directory then the program will not fail.

(obviously there is a chance even more dlls are needed and it it will fail on the nex dll, but by this time AttachProcess() has already done it's harm, right?)

Or do you know of another way to prevent say a program relying on dwmapi.dll from running on a XP system or older that has a fake dwmapi.dll in the current directory that the program's document is triggered from? (let's say by a lame user not noticing the dll residing in the same folder)

Re: Use SetDllDirectory() by default to prevent dll-hijackin

Posted: Thu Sep 16, 2010 7:02 pm
by Trond
If it's missing then yeah the OS will react with: "dtest.exe - System Error"
Yes. But how many people have their file associations set to upen with programs that don't work?

Re: Use SetDllDirectory() by default to prevent dll-hijackin

Posted: Thu Sep 16, 2010 8:10 pm
by Thorium
Rescator wrote: 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).
That would not make a difference. You could place entrys of a dummy.dll right at the top of the import table. And you can call SetDllDirectory("") inside of dummy.dll. But thats useless. A attacker can just use dummy.dll for the name of the .dll he want to get into the process. So thats no solution at all.

There is a solution i can think of but it depends on that kernel32.dll will be allways loaded from system32 on old OS's, which i am not sure of.
The solution would be complex and need some work to implement. You would need to write a tool just like UPX. That creates a new section in the .exe and writes some code to it, that will do some work on startup. Additionally moving all none-kernel32.dll imports out of the import table, so they are not processed by the PE loader.

So the PE loader will only load ntdll.dll and kernel32.dll.
The code that was been written to the .exe will then rebuild the original import table and initializes it manually.

It's possible to do it, but it's a lot of work. And it isnt needed to be a part of PB, could work with any .exe.

Re: Use SetDllDirectory() by default to prevent dll-hijackin

Posted: Thu Sep 16, 2010 10:56 pm
by PB
All this talk... and yet the solution seems to be to just put SetDllDirectory("") at
the top of your app's source code? Why not just do that, then? Seems simple!

Re: Use SetDllDirectory() by default to prevent dll-hijackin

Posted: Fri Sep 17, 2010 4:32 am
by Flower
freak wrote:>Oh, and if somebody is still using pre XP SP1, they should not be surprised if they have security problems anyway.
That is so true :mrgreen:

Re: Use SetDllDirectory() by default to prevent dll-hijackin

Posted: Fri Sep 17, 2010 5:26 am
by Thorium
PB wrote:All this talk... and yet the solution seems to be to just put SetDllDirectory("") at
the top of your app's source code? Why not just do that, then? Seems simple!
The talk is mainly about the import of dll functions. And imports are initialized befor any code of the .exe is executed. So you remove the searchpath but the dlls are allready loaded.

Re: Use SetDllDirectory() by default to prevent dll-hijackin

Posted: Fri Sep 17, 2010 11:07 pm
by Rescator
Yeah *sigh*, it seems like maybe there is no easy solution for the import stuff.

I'd still like to see some form of improvement on OpenLibrary() though, maybe using some flag or something.
Because I (as I said earlier) certainly test by "feature" rather than OS. (a practice MS recommends)
I try to open a dll, if it's found I check if the function is available, if it is then I use it,
if not then I either use a fallback procedure instead, or I simply do not use that feature on that particular machine.

This way a program that works on XP+ will take advantage of Vista or Win7 features when available.
And it's here that almost all the dll hijack exploits take their opportunity.

Here is a very quick example of what could be done to make OpenLibrary() "safe".

Code: Select all

Procedure.i NewOpenLibrary(library.i,filename$,safe.i=#True)
    Protected result.i,path$,len.i
    If safe
        path$=GetPathPart(ProgramFilename()) ;Why isn't there a ProgramFilepath() ???
        Debug path$+filename$
        result=OpenLibrary(library,path$+filename$)
        If Not result
            len=GetSystemDirectory_(@path$,0) ;TCHAR + null term length is returned.
            If len
                path$=Space(len) ;This could be len-1 but 1 extra byte buffer doesn't hurt.
                If GetSystemDirectory_(@path$,len)
                    Debug path$+"\"+filename$
                    result=OpenLibrary(library,path$+"\"+filename$)
                EndIf
            EndIf
            If Not result ;Not sure if looking in windows directory has any purpose, it's mostly a holdover from the 16bit windows days.
                len=GetWindowsDirectory_(@path$,0)
                If len
                    path$=Space(len)
                    If GetWindowsDirectory_(@path$,len)
                        Debug path$+"\"+filename$
                        result=OpenLibrary(library,path$+"\"+filename$)
                    EndIf
                EndIf
            EndIf
        EndIf
    Else
        result=OpenLibrary(library,filename$)
    EndIf
    ProcedureReturn result
EndProcedure

Debug NewOpenLibrary(#PB_Any,"testing.dll")
Debug ""
Debug NewOpenLibrary(#PB_Any,"dwmapi.dll")
Debug ""
Debug NewOpenLibrary(#PB_Any,"kernel32.dll")
I'm not sure looking in c:\windows\ for a dll makes much sense, on my Win7 I only got twain.dll there and I get an error trying to open it. (16bit?)
But looking in the program exe's path and system32 and failing if not found in either seems safe enough.
In this example I choose to look in the program's path first (to allow overriding support or using older versions of a dll, to avoid "dll hell").

(PS! if a x86 program is run on a 64bit system then system32 is secretly redirected to SysWOW64, it'll still show as system32 though)

To use the old OpenLibrary() behavior just specify #False.
setting the safe flag to False is also of use when using explicit/full paths.

I guess I could have made this more advanced by automatically detecting if you gave, a full path, or a relative path, or no path,
and behaved 3 different ways, but this was just a quick example.

I.e:
If full path is given it should just try the full path, and succeed or fail.
If relative path is given it should just try relative to the programs path, and succeed or fail.
If no path is given it should try the programs path and if fail it will then try the system32 path, and succeed or fail.

You can't get much safer than that if OpenLibrary() behaved like that by default.
Is this reasonable Fred?

Re: Use SetDllDirectory() by default to prevent dll-hijackin

Posted: Fri Sep 17, 2010 11:18 pm
by Rescator
Dammit, I just couldn't resist.
Here is an example of safemode by default and automatic detection if the dll path is, full, relative, or none.
Sadly there is quite some overhead here and can be optimized a lot, but it's a proof of concept so good enough for that purpose.

On and the benefit of this is that there is no reliance on SetDllDirectory() so this will work not just on Win7, Vista, and XP but older Windows as well.
I believe that Linux (and Mac OS X indirectly?) does not ever look in current dir or PATH var like windows does, so they may not need this "fix".
But... it might still make sense to do this on those as well as we'd have identical .dll or .so search behavior on all 3 platforms that is "safe"

Code: Select all

Procedure.i NewOpenLibrary(library.i,filename$,safe.i=#True)
    Protected result.i,path$,len.i,local.i,full.i
    If safe
        If FindString(filename$,":",1)
            full=#True
        Else
            If FindString(filename$,"\",1) Or FindString(filename$,"/",1)
                local=#True
            EndIf
        EndIf
        If full
            Debug filename$
            result=OpenLibrary(library,filename$)
        Else
            path$=GetPathPart(ProgramFilename()) ;Why isn't there a ProgramFilepath() ???
            Debug path$+filename$
            result=OpenLibrary(library,path$+filename$)
            If Not result And Not local
                len=GetSystemDirectory_(@path$,0) ;TCHAR + null term length is returned.
                If len
                    path$=Space(len) ;This could be len-1 but 1 extra byte buffer doesn't hurt.
                    If GetSystemDirectory_(@path$,len)
                        Debug path$+"\"+filename$
                        result=OpenLibrary(library,path$+"\"+filename$)
                    EndIf
                EndIf
            EndIf
        EndIf
    Else
        result=OpenLibrary(library,filename$)
    EndIf
    ProcedureReturn result
EndProcedure

Debug NewOpenLibrary(#PB_Any,"plugin\testing.dll")
Debug ""
Debug NewOpenLibrary(#PB_Any,"dwmapi.dll")
Debug ""
Debug NewOpenLibrary(#PB_Any,"c:\windows\system32\kernel32.dll")