diff --git a/wgengine/router/router_windows.go b/wgengine/router/router_windows.go index 0f01194d3..73d219bce 100644 --- a/wgengine/router/router_windows.go +++ b/wgengine/router/router_windows.go @@ -12,6 +12,7 @@ "net/netip" "os" "os/exec" + "path/filepath" "slices" "strings" "sync" @@ -155,6 +156,13 @@ type firewallTweaker struct { // stop makes fwProc exit when closed. fwProcWriter io.WriteCloser fwProcEncoder *json.Encoder + + // The path to the 'netsh.exe' binary, populated during the first call + // to runFirewall. + // + // not protected by mu; netshPath is only mutated inside netshPathOnce + netshPathOnce sync.Once + netshPath string } func (ft *firewallTweaker) clear() { ft.set(nil, nil, nil) } @@ -185,10 +193,43 @@ func (ft *firewallTweaker) set(cidrs []string, routes, localRoutes []netip.Prefi go ft.doAsyncSet() } +// getNetshPath returns the path that should be used to execute netsh. +// +// We've seen a report from a customer that we're triggering the "cannot run +// executable found relative to current directory" protection that was added to +// prevent running possibly attacker-controlled binaries. To mitigate this, +// first try looking up the path to netsh.exe in the System32 directory +// explicitly, and then fall back to the prior behaviour of passing "netsh" to +// os/exec.Command. +func (ft *firewallTweaker) getNetshPath() string { + ft.netshPathOnce.Do(func() { + // The default value is the old approach: just run "netsh" and + // let os/exec resolve that into a full path. + ft.netshPath = "netsh" + + path, err := windows.KnownFolderPath(windows.FOLDERID_System, 0) + if err != nil { + ft.logf("getNetshPath: error getting FOLDERID_System: %v", err) + return + } + + expath := filepath.Join(path, "netsh.exe") + if _, err := os.Stat(expath); err == nil { + ft.netshPath = expath + return + } else if !os.IsNotExist(err) { + ft.logf("getNetshPath: error checking for existence of %q: %v", expath, err) + } + + // Keep default + }) + return ft.netshPath +} + func (ft *firewallTweaker) runFirewall(args ...string) (time.Duration, error) { t0 := time.Now() args = append([]string{"advfirewall", "firewall"}, args...) - cmd := exec.Command("netsh", args...) + cmd := exec.Command(ft.getNetshPath(), args...) cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true} b, err := cmd.CombinedOutput() if err != nil { diff --git a/wgengine/router/router_windows_test.go b/wgengine/router/router_windows_test.go new file mode 100644 index 000000000..9989ddbc7 --- /dev/null +++ b/wgengine/router/router_windows_test.go @@ -0,0 +1,19 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package router + +import ( + "path/filepath" + "testing" +) + +func TestGetNetshPath(t *testing.T) { + ft := &firewallTweaker{ + logf: t.Logf, + } + path := ft.getNetshPath() + if !filepath.IsAbs(path) { + t.Errorf("expected absolute path for netsh.exe: %q", path) + } +}