Skip to content

Commit

Permalink
windows: remove LazyDLL calls for system modules (hashicorp#19925)
Browse files Browse the repository at this point in the history
On Windows, Nomad uses `syscall.NewLazyDLL` and `syscall.LoadDLL` functions to
load a few system DLL files, which does not prevent DLL hijacking
attacks. Hypothetically a local attacker on the client host that can place an
abusive library in a specific location could use this to escalate privileges to
the Nomad process. Although this attack does not fall within the Nomad security
model, it doesn't hurt to follow good practices here.

We can remove two of these DLL loads by using wrapper functions provided by the
stdlib in `x/sys/windows`

Co-authored-by: dduzgun-security <[email protected]>
  • Loading branch information
tgross and dduzgun-security authored Feb 9, 2024
1 parent 62c57d2 commit 110d93a
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 30 deletions.
3 changes: 3 additions & 0 deletions .changelog/19925.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:security
windows: Remove `LazyDLL` calls for system modules to harden Nomad against attacks from the host
```
31 changes: 11 additions & 20 deletions command/agent/host/windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ package host
import (
"os"
"syscall"
"unsafe"

"golang.org/x/sys/windows"
)

func uname() string {
Expand All @@ -36,34 +37,24 @@ func mountedPaths() (disks []string) {
}

type df struct {
size int64
avail int64
size uint64 // "systemFree" less quotas
avail uint64
systemFree uint64
}

func makeDf(path string) (*df, error) {
h, err := syscall.LoadDLL("kernel32.dll")
if err != nil {
return nil, err
}

c, err := h.FindProc("GetDiskFreeSpaceExW")
if err != nil {
return nil, err
}

df := &df{}
err := windows.GetDiskFreeSpaceEx(
syscall.StringToUTF16Ptr(path),
&df.avail, &df.size, &df.systemFree)

c.Call(uintptr(unsafe.Pointer(syscall.StringToUTF16Ptr(path))),
uintptr(unsafe.Pointer(&df.size)),
uintptr(unsafe.Pointer(&df.avail)))

return df, nil
return df, err
}

func (d *df) total() uint64 {
return uint64(d.size)
return d.size
}

func (d *df) available() uint64 {
return uint64(d.avail)
return d.avail
}
13 changes: 3 additions & 10 deletions drivers/shared/executor/executor_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"fmt"
"os"
"syscall"

"golang.org/x/sys/windows"
)

// configure new process group for child process
Expand Down Expand Up @@ -45,18 +47,9 @@ func (e *UniversalExecutor) killProcessTree(proc *os.Process) error {
}

// Send a Ctrl-Break signal for shutting down the process,
// like in https://golang.org/src/os/signal/signal_windows_test.go
func sendCtrlBreak(pid int) error {
dll, err := syscall.LoadDLL("kernel32.dll")
if err != nil {
return fmt.Errorf("Error loading kernel32.dll: %v", err)
}
proc, err := dll.FindProc("GenerateConsoleCtrlEvent")
err := windows.GenerateConsoleCtrlEvent(syscall.CTRL_BREAK_EVENT, uint32(pid))
if err != nil {
return fmt.Errorf("Cannot find procedure GenerateConsoleCtrlEvent: %v", err)
}
result, _, err := proc.Call(syscall.CTRL_BREAK_EVENT, uintptr(pid))
if result == 0 {
return fmt.Errorf("Error sending ctrl-break event: %v", err)
}
return nil
Expand Down

0 comments on commit 110d93a

Please sign in to comment.