runtime: Windows DLL preloading attack possible for winmm.dll

[sj26]

Go 1.11 seems vulnerable to dll preloading on windows with winmm.dll. It looks like #14959 mostly fixed this, kernel32.dll etc are protected, but winmm.dll still seems to be affected. It seems to be loaded implicitly by the go runtime — https://github.com/golang/go/blob/6174b5e21e73714c63061e66efdbe180e1c5491d/src/pkg/runtime/thread_windows.c#L31 — but I notice is not listed with the other safely loaded DLLs — https://github.com/golang/go/blob/6174b5e21e73714c63061e66efdbe180e1c5491d/src/pkg/syscall/zsyscall_windows_amd64.go#L9-L19

What version of Go are you using (go version)?

$ docker run --rm golang:1.11 go version
go version go1.11.2 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

$ docker run --rm golang:1.11 go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build782466443=/tmp/go-build -gno-record-gcc-switches"

What did you do?

1. Create a main.go with:

   package main
   
   import "fmt"
   
   func main() {
   fmt.Println("Hello world")
   }

2. Cross-compile for windows:

   docker run -v $PWD:/go -e GOARCH=amd64 -e GOOS=windows --rm golang:1.11 go build -o test.exe main.go

3. Copy test.exe to a windows vm

4. Add a winmm.dll beside test.exe with contents not a dll

5. Double click test.exe

What did you expect to see?

"Hello world"

What did you see instead?

[odeke-em]

Thank you for filing this bug @sj26 !

I'll page some folks who can help both with Windows and security /cc @alexbrainman @bradfitz @jordanrh1 @FiloSottile @aclements @ianlancetaylor

In the future @sj26, please report security bugs to security@golang.org as recommended at https://golang.org/security

[alexbrainman]

It looks like #14959 mostly fixed this, kernel32.dll etc are protected, but winmm.dll still seems to be affected. It seems to be loaded implicitly by the go runtime —

kernel32.dll also loaded by runtime - see many lines ending with "kernel32.dll" in os_windows.go file. Going by your logic, kernel32.dll must be as broken as winmm.dll. I suspect, you can do your trick with kernel32.dll, if you make your program more complicated.

I tried your trick with notepad.exe and corrupted kernel32.dll, and notepad.exe does not start. And I do not see much difference between notepad.exe and Go built executable. They both have import table in the executable that specifies which DLL and functions to use. And Windows program loader searches for DLLs as per standard rules. How else do you propose Windows executable access OS?

Leaving this to real security experts.

Alex

[sj26]

I confess I don't use Windows and don't understand its dynamic library loading in detail. I'm reporting an issue we were alerted to which seemed odd. It sounds like a bug that some libraries would be protected and some would not. I didn't consider it a high severity security issue, it just seemed like a library search order problem.

kernel32.dll does not seem affected, nor any other dll I could find referenced, only winmm.dll.

The only difference I could find is that unaffected dlls are referenced in that generated syscall source file, and then again referenced in runtime, but winmm is only referenced in runtime. Perhaps this changes when and how they are loaded?

I notice that back in #14959 the effort was to switch these well-known libraries to use LoadLibraryEx with a flag to make sure they were loaded only from the system directory, not the regular search path. I don't understand the codebase deeply enough, but I couldn't find evidence of this any more, I can only find LoadLibrary[A|W]. So I'm not sure how only winmm.dll seems affected.

Perhaps this is because of Windows' underlying "known dlls" functionality?

I'll leave the sleuthing to folks who understand the functionality. Maybe the answer is that this is not a problem, it's just how Windows functions?

[diagprov]

Hi guys, I've been a Windows dev for some time and I do stuff in security. I'm not affiliated with the project, I just stumbled upon this bug looking for another issue.

So, the default DLL search path starts with the application directory itself by default. This is because Windows considers application directories to be bundles, much like OSX .app folders: https://blogs.msdn.microsoft.com/oldnewthing/20110620-00/?p=10393 and https://blogs.msdn.microsoft.com/oldnewthing/20161013-00/?p=94505

KnownDLLs was a Longhorn (Vista) optimization, which loads the listed DLLs into memory at boot and then every time an application asks for one the DLL is mapped in automatically, without ever searching the disk for the file and running the PE loader again. You can probably see why this is a performance optimization, avoiding these disk hits for every process. This has some security implications, as it means windows ignores the usual search paths, so you won't be able to inject kernel32.dll even if you try really hard. It is not, however, a security boundary or defensive measure (it just has useful consequences).

There's also another method that is rarely discussed, WinSxS. Most PE binaries (especially those that implement GUIs) have an application manifest xml file embedded as a resource. dot net developers will be more familiar with this, I suspect. This XML resource can specify alternative versions of DLLs (assemblies) to be loaded from the WinSxS directory. In fact, kernel32.dll is just hardlinked into system32. This is an aside, but I want to mention it for completeness.

So, now we come to how DLLs actually get loaded. There are two ways: statically, from the import directory table. In this case, the load order is the default supplied by Windows and you can't change it. The other case is dynamically, your LoadLibrary* or dlopen. This you can alter the order at runtime.

The pragma dynimport above is creating a static, embedded in the PE IDT reference to winmm.dll; here's the output from dumpbin (windows objdump):

V:\go\src\test\testapp>dumpbin /imports testapp.exe
Microsoft (R) COFF/PE Dumper Version 14.16.27023.1
Copyright (C) Microsoft Corporation.  All rights reserved.

Dump of file testapp.exe

File Type: EXECUTABLE IMAGE

  Section contains the following imports:

    winmm.dll
                548000 Import Address Table
                5DF2F6 Import Name Table
                     0 time date stamp
                     0 Index of first forwarder reference

                           0 timeEndPeriod
                           0 timeBeginPeriod

    ws2_32.dll
                548018 Import Address Table
                5DF30E Import Name Table
                     0 time date stamp
                     0 Index of first forwarder reference

                           0 WSAGetOverlappedResult

    kernel32.dll
                548028 Import Address Table
                5DF31E Import Name Table
                     0 time date stamp
                     0 Index of first forwarder reference

                           0 WriteFile
                           0 WriteConsoleW
                           0 WaitForSingleObject
<snip>

As a consequence, there's nothing the go runtime code can do to alter the search path, because no go runtime code can possibly be called before that DLL is resolved and loaded by Windows. The only alternative is to load the library dynamically.

A security issue arises with DLL loading when an application is executed either with higher privileges, or with different privileges, and the application directory is writable. In this case, where such static DLLs are loaded and they're not KnownDLLs, DLL Injection is trivial and you have a local privesg bug (or lateral movement). As I said above, Windows considers directories to be application bundles and for them to be safe locations to load libraries from (installers in the NSIS case are a bit of a special case, because they're not run from trusted locations). The privileges of such a directory should be set appropriately by code installing the binary in question. I suggest using WIX and generating an MSI; Windows Installer will then do this for you.

There are other ways a developer may shoot themselves in the foot in addition to this DLL. I am assuming any cgo C code that did some kind of pragma import would have that honoured and the DLL added to the IDT as well (I have not looked into this). Same problem occurs. Go plugins also produce loadable DLL components out of go code and developers have plenty of opportunity to do something inadvisable with those.

I don't really have strong opinions as to what to do about this. It really depends on if you want to support writing application installers in golang. However, as things stand this is totally expected behaviour and would apply to any non-KnownDLL that is referenced in the import table.

[bradfitz]

@diagprov, thanks for the information!

[sj26]

@diagprov incredibly interesting, thank you!

I think a common use case for golang is writing utility programs which are often downloaded directly via a browser to a Downloads directory and then directly executed in that directory. Which creates an opportunity to persuade a user to download a maliciously crafted and appropriately named dll as well in the same way, which would then be in the same directory. But I'm not sure there's anything practical that golang can do here? It sounds like using the import table for kernel32.dll is pretty safe, and that exposes LoadLibrary etc which can be limited to system load paths which could then be used to dynamically load the rest of the required libraries, but this sounds pretty convoluted, and not always desirable.

[diagprov]

So just quickly, on linux systems, the "API" linux applications can expect to remain consistent and mostly working is directly at the syscall level. The syscall numbers are stable and if your application calls them directly and statically links (like go does), fine. This is not true of Windows, since NT supports multiple subsystems - the syscall interface is an internal API, as is NTDLL.DLL. The only subsystem of this kind (WSL works differently) still used is "win32"; public API for that is kernel32.dll, user32.dll, advapi32.dll, etc etc. You more or less have to link against kernel32.dll and possibly others from KnownDLLs. This is OK, it's how Windows works.

I don't think there's anything wrong with LoadLibrary/handle check/GetProcAddress/null check/use function as a way to load winmm. It's a bit annoying perhaps, but the NewLazyDLL code in Go I believe already uses this pattern (you can call LoadLibrary on a DLL loaded through the IDT no problem, you'll get a handle like normal, just to head off that question). You can control the search path this way and allow the static imports by the runtime to be only "standard" DLLs.

One possible question is why Golang is raising timer resolution frequency in the first place. I would guess this is to make the GC as performant as possible, but this has global impact, so, for the life of any golang process the timer resolution goes from the default 15.6ms to 1ms for the whole machine. This would be of particular concern from windows services ("daemons") written in golang. Chrome have discussed it: https://bugs.chromium.org/p/chromium/issues/detail?id=153139 and MSDN also discusses the performance considerations, to quote:

This function affects a global Windows setting. Windows uses the lowest value (that is, highest resolution) requested by any process. Setting a higher resolution can improve the accuracy of time-out intervals in wait functions. However, it can also reduce overall system performance, because the thread scheduler switches tasks more often. High resolutions can also prevent the CPU power management system from entering power-saving modes.

[ianlancetaylor]

For background on why the Go runtime fiddles with the timer resolution frequency see #8687 and #20937. It's not because of the garbage collector.

[diagprov]

So it seems the calls were removed, but it slows down go's scheduling code in some cases, so the removal was reverted. #7876 seems to be an open issue for removing these calls. More complicated than I thought and unfortunately I don't know go nearly well enough to help on that issue.

[sj26]

It sounds like we're stuck with timeBeginPeriod for now, although there might be a better solution in the works.

In the meantime, could this library be imported dynamically with the system-only flags right before the timeBeginPeriod call? That would solve this particular issue with hopefully minimal performance impact?

[sj26]

Wow, I was looking at a very old version of the code. Most of the code I posted is very out of date.

I had a go at making winmm load dynamically and safely using LoadLibraryEx with LOAD_LIBRARY_SEARCH_SYSTEM32. It works!

https://github.com/golang/go/compare/master...sj26:load-winmm-dynamically

This approach could be applied to the other LoadLibraryA calls here, too.

[alexbrainman]

In the meantime, could this library be imported dynamically with the system-only flags right before the timeBeginPeriod call? That would solve this particular issue with hopefully minimal performance impact?

Personally I don't feel there is a problem here.

Alex

[gopherbot]

Change https://golang.org/cl/165798 mentions this issue: runtime: safely load DLLs

[gopherbot]

Change https://golang.org/cl/168339 mentions this issue: [release-branch.go1.12] runtime: safely load DLLs

[gopherbot]

Change https://golang.org/cl/175378 mentions this issue: [release-branch.go1.11] runtime: safely load DLLs