Skip to content

Commit ca57180

Browse files
mdempskymknyszek
authored andcommitted
[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 <[email protected]> Run-TryBot: Matthew Dempsky <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Damien Neil <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/go/+/447875 Run-TryBot: David du Colombier <[email protected]>
1 parent d9c62ce commit ca57180

File tree

3 files changed

+20
-6
lines changed

3 files changed

+20
-6
lines changed

src/os/exec/env_test.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
func TestDedupEnv(t *testing.T) {
1313
tests := []struct {
1414
noCase bool
15+
nulOK bool
1516
in []string
1617
want []string
1718
wantErr bool
@@ -36,9 +37,15 @@ func TestDedupEnv(t *testing.T) {
3637
want: []string{"B=b"},
3738
wantErr: true,
3839
},
40+
{
41+
// Plan 9 needs to preserve environment variables with NUL (#56544).
42+
nulOK: true,
43+
in: []string{"path=one\x00two"},
44+
want: []string{"path=one\x00two"},
45+
},
3946
}
4047
for _, tt := range tests {
41-
got, err := dedupEnvCase(tt.noCase, tt.in)
48+
got, err := dedupEnvCase(tt.noCase, tt.nulOK, tt.in)
4249
if !reflect.DeepEqual(got, tt.want) || (err != nil) != tt.wantErr {
4350
t.Errorf("Dedup(%v, %q) = %q, %v; want %q, error:%v", tt.noCase, tt.in, got, err, tt.want, tt.wantErr)
4451
}

src/os/exec/exec.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -745,23 +745,27 @@ func minInt(a, b int) int {
745745
// dedupEnv returns a copy of env with any duplicates removed, in favor of
746746
// later values.
747747
// Items not of the normal environment "key=value" form are preserved unchanged.
748-
// Items containing NUL characters are removed, and an error is returned along with
749-
// the remaining values.
748+
// Except on Plan 9, items containing NUL characters are removed, and
749+
// an error is returned along with the remaining values.
750750
func dedupEnv(env []string) ([]string, error) {
751-
return dedupEnvCase(runtime.GOOS == "windows", env)
751+
return dedupEnvCase(runtime.GOOS == "windows", runtime.GOOS == "plan9", env)
752752
}
753753

754754
// dedupEnvCase is dedupEnv with a case option for testing.
755755
// If caseInsensitive is true, the case of keys is ignored.
756-
func dedupEnvCase(caseInsensitive bool, env []string) ([]string, error) {
756+
// If nulOK is false, items containing NUL characters are allowed.
757+
func dedupEnvCase(caseInsensitive, nulOK bool, env []string) ([]string, error) {
757758
var err error
758759
out := make([]string, 0, len(env))
759760
saw := make(map[string]int, len(env)) // key => index into out
760761
for _, kv := range env {
761-
if strings.IndexByte(kv, 0) != -1 {
762+
// Reject NUL in environment variables to prevent security issues (#56284);
763+
// except on Plan 9, which uses NUL as os.PathListSeparator (#56544).
764+
if !nulOK && strings.IndexByte(kv, 0) != -1 {
762765
err = errors.New("exec: environment variable contains NUL")
763766
continue
764767
}
768+
765769
k, _, ok := strings.Cut(kv, "=")
766770
if !ok {
767771
out = append(out, kv)

src/os/exec/exec_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1030,6 +1030,9 @@ func TestDedupEnvEcho(t *testing.T) {
10301030
}
10311031

10321032
func TestEnvNULCharacter(t *testing.T) {
1033+
if runtime.GOOS == "plan9" {
1034+
t.Skip("plan9 explicitly allows NUL in the enviroment")
1035+
}
10331036
cmd := helperCommand(t, "echoenv", "FOO", "BAR")
10341037
cmd.Env = append(cmd.Env, "FOO=foo\x00BAR=bar")
10351038
out, err := cmd.CombinedOutput()

0 commit comments

Comments
 (0)