[release-branch.go1.18] 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 #56550.

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/+/447875
Run-TryBot: David du Colombier <0intro@gmail.com>

3 files changed, 20 insertions, 6 deletions

diff --git a/src/os/exec/env_test.go b/src/os/exec/env_test.go
index 47b7c04705..43d14fb56d 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
@@ -36,9 +37,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 056a7e07d7..e1147b75fa 100644
--- a/src/os/exec/exec.go
+++ b/src/os/exec/exec.go
@@ -745,23 +745,27 @@ 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.
-// 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) {
 	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 {
+		// 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
 		}
+
 		k, _, ok := strings.Cut(kv, "=")
 		if !ok {
 			out = append(out, kv)
diff --git a/src/os/exec/exec_test.go b/src/os/exec/exec_test.go
index 0be8c6cc18..99bc0329d1 100644
--- a/src/os/exec/exec_test.go
+++ b/src/os/exec/exec_test.go
@@ -1030,6 +1030,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.Env, "FOO=foo\x00BAR=bar")
 	out, err := cmd.CombinedOutput()