1
0
mirror of https://github.com/golang/go synced 2024-11-17 12:34:46 -07:00

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.
Fixes #56544.

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>
This commit is contained in:
Matthew Dempsky 2022-11-03 11:02:51 -07:00
parent c1c6b0ca79
commit bc2dc23846
3 changed files with 20 additions and 6 deletions

View File

@ -14,6 +14,7 @@ func TestDedupEnv(t *testing.T) {
tests := []struct {
noCase bool
nulOK bool
in []string
want []string
wantErr bool
@ -50,9 +51,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)
}

View File

@ -1208,15 +1208,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
@ -1225,10 +1226,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.

View File

@ -1038,6 +1038,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()