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

diff --git a/src/os/exec/env_test.go b/src/os/exec/env_test.go
index 126ec83cedf45c777a27ad17fcb4bae0f9824e9f..b9e09247febb557d021b13101d751089ee6ad5c6 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
@@ -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 d3c4e7ae83b56176d0f01fbc06e04125758e2556..9b19cb648775847df15a82dfd2be7f1947857613 100644 (file)
--- 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 d377f85934053618eec8fad7f1a2d2c780046586..8859bd18fcb59b517127050aac7fbad15f8cd802 100644 (file)
--- 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()