diff options
| -rw-r--r-- | src/os/exec/env_test.go | 19 | ||||
| -rw-r--r-- | src/os/exec/exec.go | 19 | ||||
| -rw-r--r-- | src/os/exec/exec_test.go | 9 | ||||
| -rw-r--r-- | src/syscall/exec_windows.go | 20 |
4 files changed, 52 insertions, 15 deletions
diff --git a/src/os/exec/env_test.go b/src/os/exec/env_test.go index b5ac398c27..47b7c04705 100644 --- a/src/os/exec/env_test.go +++ b/src/os/exec/env_test.go @@ -11,9 +11,10 @@ import ( func TestDedupEnv(t *testing.T) { tests := []struct { - noCase bool - in []string - want []string + noCase bool + in []string + want []string + wantErr bool }{ { noCase: true, @@ -29,11 +30,17 @@ func TestDedupEnv(t *testing.T) { in: []string{"=a", "=b", "foo", "bar"}, want: []string{"=b", "foo", "bar"}, }, + { + // Filter out entries containing NULs. + in: []string{"A=a\x00b", "B=b", "C\x00C=c"}, + want: []string{"B=b"}, + wantErr: true, + }, } for _, tt := range tests { - got := dedupEnvCase(tt.noCase, tt.in) - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("Dedup(%v, %q) = %q; want %q", tt.noCase, tt.in, got, tt.want) + got, err := dedupEnvCase(tt.noCase, tt.in) + if !reflect.DeepEqual(got, tt.want) || (err != nil) != tt.wantErr { + t.Errorf("Dedup(%v, %q) = %q, %v; want %q, error:%v", tt.noCase, tt.in, got, err, tt.want, tt.wantErr) } } } diff --git a/src/os/exec/exec.go b/src/os/exec/exec.go index ecddee690d..056a7e07d7 100644 --- a/src/os/exec/exec.go +++ b/src/os/exec/exec.go @@ -422,10 +422,14 @@ func (c *Cmd) Start() error { return err } + env, err := dedupEnv(envv) + if err != nil { + return err + } c.Process, err = os.StartProcess(c.Path, c.argv(), &os.ProcAttr{ Dir: c.Dir, Files: c.childFiles, - Env: addCriticalEnv(dedupEnv(envv)), + Env: addCriticalEnv(env), Sys: c.SysProcAttr, }) if err != nil { @@ -741,16 +745,23 @@ func minInt(a, b int) int { // dedupEnv returns a copy of env with any duplicates removed, in favor of // later values. // Items not of the normal environment "key=value" form are preserved unchanged. -func dedupEnv(env []string) []string { +// Items containing NUL characters are removed, and an error is returned along with +// the remaining values. +func dedupEnv(env []string) ([]string, error) { return dedupEnvCase(runtime.GOOS == "windows", env) } // dedupEnvCase is dedupEnv with a case option for testing. // If caseInsensitive is true, the case of keys is ignored. -func dedupEnvCase(caseInsensitive bool, env []string) []string { +func dedupEnvCase(caseInsensitive bool, env []string) ([]string, error) { + var err error out := make([]string, 0, len(env)) saw := make(map[string]int, len(env)) // key => index into out for _, kv := range env { + if strings.IndexByte(kv, 0) != -1 { + err = errors.New("exec: environment variable contains NUL") + continue + } k, _, ok := strings.Cut(kv, "=") if !ok { out = append(out, kv) @@ -766,7 +777,7 @@ func dedupEnvCase(caseInsensitive bool, env []string) []string { saw[k] = len(out) out = append(out, kv) } - return out + return out, err } // addCriticalEnv adds any critical environment variables that are required diff --git a/src/os/exec/exec_test.go b/src/os/exec/exec_test.go index 1913ae81c8..0be8c6cc18 100644 --- a/src/os/exec/exec_test.go +++ b/src/os/exec/exec_test.go @@ -1029,6 +1029,15 @@ func TestDedupEnvEcho(t *testing.T) { } } +func TestEnvNULCharacter(t *testing.T) { + cmd := helperCommand(t, "echoenv", "FOO", "BAR") + cmd.Env = append(cmd.Env, "FOO=foo\x00BAR=bar") + out, err := cmd.CombinedOutput() + if err == nil { + t.Errorf("output = %q; want error", string(out)) + } +} + func TestString(t *testing.T) { echoPath, err := exec.LookPath("echo") if err != nil { diff --git a/src/syscall/exec_windows.go b/src/syscall/exec_windows.go index 9d10d6a512..50892bee44 100644 --- a/src/syscall/exec_windows.go +++ b/src/syscall/exec_windows.go @@ -7,6 +7,7 @@ package syscall import ( + "internal/bytealg" "runtime" "sync" "unicode/utf16" @@ -115,12 +116,16 @@ func makeCmdLine(args []string) string { // the representation required by CreateProcess: a sequence of NUL // terminated strings followed by a nil. // Last bytes are two UCS-2 NULs, or four NUL bytes. -func createEnvBlock(envv []string) *uint16 { +// If any string contains a NUL, it returns (nil, EINVAL). +func createEnvBlock(envv []string) (*uint16, error) { if len(envv) == 0 { - return &utf16.Encode([]rune("\x00\x00"))[0] + return &utf16.Encode([]rune("\x00\x00"))[0], nil } length := 0 for _, s := range envv { + if bytealg.IndexByteString(s, 0) != -1 { + return nil, EINVAL + } length += len(s) + 1 } length += 1 @@ -135,7 +140,7 @@ func createEnvBlock(envv []string) *uint16 { } copy(b[i:i+1], []byte{0}) - return &utf16.Encode([]rune(string(b)))[0] + return &utf16.Encode([]rune(string(b)))[0], nil } func CloseOnExec(fd Handle) { @@ -400,12 +405,17 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle } } + envBlock, err := createEnvBlock(attr.Env) + if err != nil { + return 0, 0, err + } + pi := new(ProcessInformation) flags := sys.CreationFlags | CREATE_UNICODE_ENVIRONMENT | _EXTENDED_STARTUPINFO_PRESENT if sys.Token != 0 { - err = CreateProcessAsUser(sys.Token, argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, willInheritHandles, flags, createEnvBlock(attr.Env), dirp, &si.StartupInfo, pi) + err = CreateProcessAsUser(sys.Token, argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, willInheritHandles, flags, envBlock, dirp, &si.StartupInfo, pi) } else { - err = CreateProcess(argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, willInheritHandles, flags, createEnvBlock(attr.Env), dirp, &si.StartupInfo, pi) + err = CreateProcess(argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, willInheritHandles, flags, envBlock, dirp, &si.StartupInfo, pi) } if err != nil { return 0, 0, err |