[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>

diff --git a/src/os/exec/env_test.go b/src/os/exec/env_test.go
index 47b7c04705729002f5a918866a0967f006eb8ad1..43d14fb56dd1203bca8aea414a99e896df1f8a60 100644 (file)
--- 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 056a7e07d73ecd8edd9182f4ce372b95f389af46..e1147b75fa59e420ca4b9b5a48c4aec70c5855fc 100644 (file)
--- 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 0be8c6cc18d33b6c8e5c83cdaa83e99e274cf828..99bc0329d12c633f4051eac2ab61e1d7255e2cad 100644 (file)
--- 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()