Skip to content

Commit

Permalink
windows os/exec build checker (#24736)
Browse files Browse the repository at this point in the history
  • Loading branch information
mmaxim committed Dec 16, 2021
1 parent f263457 commit 9b33c92
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 0 deletions.
8 changes: 8 additions & 0 deletions packaging/windows/build_prerelease.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@

echo GOPATH %GOPATH%

:: check os/exec path fix is in place
pushd %GOPATH%\src\github.com\keybase\client\packaging\windows
go run osexeccheck.go
IF %ERRORLEVEL% NEQ 0 (
EXIT /B 1
)
popd

:: CGO causes dll loading security vulnerabilities
set CGO_ENABLED=0
go env
Expand Down
88 changes: 88 additions & 0 deletions packaging/windows/osexeccheck.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// On Windows, the default algorithm for searching for executable files involves looking in the current
// directory first. Go reproduces this behavior in order to stay true to how the underlying OS works.
// However, looking for executables in this order is a security problem, since attackers can induce
// an admin to invoke a rogue script by naming it with a common executable name, like git or go. If the attacker
// manages to get an admin to run `git` in a directory in which they have placed their own git.bat file
// (which could do anything), then they have exploited the system.
//
// We want to ensure that when building our own Windows distribution of Keybase, that the behavior of looking
// in the current diectory for an executable first is disabled. This test ensures that the os/exec package
// has been suitably modified to get the correct more secure behavior.
//
// In order to fix an error from this checker, you must patch the os/exec package on the machine where this
// check runs to remove the current directory search path. See https://go.dev/blog/path-security for more
// discussion. Below is a patch showing the required change:
//
// diff --git a/src/os/exec/lp_windows.go b/src/os/exec/lp_windows.go
// index e7a2cdf142..f4f0bec172 100644
// --- a/src/os/exec/lp_windows.go
// +++ b/src/os/exec/lp_windows.go
// @@ -81,9 +81,6 @@ func LookPath(file string) (string, error) {
// return "", &Error{file, err}
// }
// }
// - if f, err := findExecutable(filepath.Join(".", file), exts); err == nil {
// - return f, nil
// - }
// path := os.Getenv("path")
// for _, dir := range filepath.SplitList(path) {
// if f, err := findExecutable(filepath.Join(dir, file), exts); err == nil {

package main

import (
"fmt"
"log"
"os"
"os/exec"
"path/filepath"
"strings"
)

const FailureOutput = "check failed"

func createBatFile() (string, error) {
wd, err := os.Getwd()
if err != nil {
return "", fmt.Errorf("failed to get working directory: %w", err)
}
execPath := filepath.Join(wd, "go.bat")
if err := os.WriteFile(execPath, []byte("@echo "+FailureOutput), 0777); err != nil {
return "", fmt.Errorf("failed to write bat file: %w", err)
}
return execPath, nil
}

func execGoCommand() error {
cmd := exec.Command("go", "version")
outbytes, err := cmd.CombinedOutput()
if err != nil {
return fmt.Errorf("failed to run go command: %w", err)
}
out := strings.TrimSpace(string(outbytes[:]))
log.Printf("output: %s", out)
if out == FailureOutput {
return fmt.Errorf("Ran go.bat instead of go! In order to fix see the top comment in this source file")
}
return nil
}

func run() error {
path, err := createBatFile()
if err != nil {
log.Printf("error creating bat file: %s", err)
return err
}
defer os.Remove(path)
if err := execGoCommand(); err != nil {
log.Printf("failed to execute go command: %s", err)
return err
}
return nil
}

func main() {
if err := run(); err != nil {
os.Exit(3)
}
}

0 comments on commit 9b33c92

Please sign in to comment.