From 110d93ab25ae1c2f3711fd925951cef2af767277 Mon Sep 17 00:00:00 2001
From: Tim Gross <tgross@hashicorp.com>
Date: Fri, 9 Feb 2024 08:47:48 -0500
Subject: [PATCH] windows: remove LazyDLL calls for system modules (#19925)

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 <deniz.duzgun@hashicorp.com>
---
 .changelog/19925.txt                        |  3 ++
 command/agent/host/windows.go               | 31 ++++++++-------------
 drivers/shared/executor/executor_windows.go | 13 ++-------
 3 files changed, 17 insertions(+), 30 deletions(-)
 create mode 100644 .changelog/19925.txt

diff --git a/.changelog/19925.txt b/.changelog/19925.txt
new file mode 100644
index 000000000..a9508d1bc
--- /dev/null
+++ b/.changelog/19925.txt
@@ -0,0 +1,3 @@
+```release-note:security
+windows: Remove `LazyDLL` calls for system modules to harden Nomad against attacks from the host
+```
diff --git a/command/agent/host/windows.go b/command/agent/host/windows.go
index 1631cdc08..8e623accf 100644
--- a/command/agent/host/windows.go
+++ b/command/agent/host/windows.go
@@ -9,7 +9,8 @@ package host
 import (
 	"os"
 	"syscall"
-	"unsafe"
+
+	"golang.org/x/sys/windows"
 )
 
 func uname() string {
@@ -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
 }
diff --git a/drivers/shared/executor/executor_windows.go b/drivers/shared/executor/executor_windows.go
index d12ba14a1..457f29a6e 100644
--- a/drivers/shared/executor/executor_windows.go
+++ b/drivers/shared/executor/executor_windows.go
@@ -9,6 +9,8 @@ import (
 	"fmt"
 	"os"
 	"syscall"
+
+	"golang.org/x/sys/windows"
 )
 
 // configure new process group for child process
@@ -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")
+	err := windows.GenerateConsoleCtrlEvent(syscall.CTRL_BREAK_EVENT, uint32(pid))
 	if err != nil {
-		return fmt.Errorf("Error loading kernel32.dll: %v", err)
-	}
-	proc, err := dll.FindProc("GenerateConsoleCtrlEvent")
-	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