diff options
author | Matthew Dempsky <mdempsky@google.com> | 2022-11-03 11:02:51 -0700 |
---|---|---|
committer | Michael Knyszek <mknyszek@google.com> | 2022-11-09 18:43:59 +0000 |
commit | a2335d052bbd7cadebd09a5bc378d2110c280fd7 (patch) | |
tree | a7da2ec5280a4775a1c984ec82161ece919b658d | |
parent | 39ac1fbd1393deed8888245dcf653220b14376f6 (diff) | |
download | go-a2335d052bbd7cadebd09a5bc378d2110c280fd7.tar.gz go-a2335d052bbd7cadebd09a5bc378d2110c280fd7.zip |
[release-branch.go1.19] os/exec: allow NUL in environment variables on Plan 9
Plan 9 uses NUL as os.PathListSeparator, so it's almost always going
to appear in the environment variable list. Exempt GOOS=plan9 from the
check for NUL in environment variables.
For #56284.
For #56544.
Fixes #56551.
Change-Id: I23df233cdf20c0a9a606fd9253e15a9b5482575a
Reviewed-on: https://go-review.googlesource.com/c/go/+/447715
Reviewed-by: David du Colombier <0intro@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/447799
Run-TryBot: David du Colombier <0intro@gmail.com>
-rw-r--r-- | src/os/exec/env_test.go | 9 | ||||
-rw-r--r-- | src/os/exec/exec.go | 14 | ||||
-rw-r--r-- | src/os/exec/exec_test.go | 3 |
3 files changed, 20 insertions, 6 deletions
diff --git a/src/os/exec/env_test.go b/src/os/exec/env_test.go index 126ec83ced..b9e09247fe 100644 --- a/src/os/exec/env_test.go +++ b/src/os/exec/env_test.go @@ -12,6 +12,7 @@ import ( func TestDedupEnv(t *testing.T) { tests := []struct { noCase bool + nulOK bool in []string want []string wantErr bool @@ -48,9 +49,15 @@ func TestDedupEnv(t *testing.T) { want: []string{"B=b"}, wantErr: true, }, + { + // Plan 9 needs to preserve environment variables with NUL (#56544). + nulOK: true, + in: []string{"path=one\x00two"}, + want: []string{"path=one\x00two"}, + }, } for _, tt := range tests { - got, err := dedupEnvCase(tt.noCase, tt.in) + got, err := dedupEnvCase(tt.noCase, tt.nulOK, 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 d3c4e7ae83..9b19cb6487 100644 --- a/src/os/exec/exec.go +++ b/src/os/exec/exec.go @@ -930,15 +930,16 @@ func (c *Cmd) Environ() []string { // 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. -// Items containing NUL characters are removed, and an error is returned along with -// the remaining values. +// Except on Plan 9, 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) + return dedupEnvCase(runtime.GOOS == "windows", runtime.GOOS == "plan9", 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, error) { +// If nulOK is false, items containing NUL characters are allowed. +func dedupEnvCase(caseInsensitive, nulOK bool, env []string) ([]string, error) { // Construct the output in reverse order, to preserve the // last occurrence of each key. var err error @@ -947,10 +948,13 @@ func dedupEnvCase(caseInsensitive bool, env []string) ([]string, error) { for n := len(env); n > 0; n-- { kv := env[n-1] - if strings.IndexByte(kv, 0) != -1 { + // Reject NUL in environment variables to prevent security issues (#56284); + // except on Plan 9, which uses NUL as os.PathListSeparator (#56544). + if !nulOK && strings.IndexByte(kv, 0) != -1 { err = errors.New("exec: environment variable contains NUL") continue } + i := strings.Index(kv, "=") if i == 0 { // We observe in practice keys with a single leading "=" on Windows. diff --git a/src/os/exec/exec_test.go b/src/os/exec/exec_test.go index d377f85934..8859bd18fc 100644 --- a/src/os/exec/exec_test.go +++ b/src/os/exec/exec_test.go @@ -1054,6 +1054,9 @@ func TestDedupEnvEcho(t *testing.T) { } func TestEnvNULCharacter(t *testing.T) { + if runtime.GOOS == "plan9" { + t.Skip("plan9 explicitly allows NUL in the enviroment") + } cmd := helperCommand(t, "echoenv", "FOO", "BAR") cmd.Env = append(cmd.Environ(), "FOO=foo\x00BAR=bar") out, err := cmd.CombinedOutput() |