mirror of
https://github.com/golang/go
synced 2024-09-29 22:34:33 -06:00
syscall, os/exec: reject environment variables containing NULs
Check for and reject environment variables containing NULs. The conventions for passing environment variables to subprocesses cause most or all systems to interpret a NUL as a separator. The syscall package rejects environment variables containing a NUL on most systems, but erroniously did not do so on Windows. This causes an environment variable such as "FOO=a\x00BAR=b" to be interpreted as "FOO=a", "BAR=b". Check for and reject NULs in environment variables passed to syscall.StartProcess on Windows. Add a redundant check to os/exec as extra insurance. Fixes #56284 Fixes CVE-2022-41716 Change-Id: I2950e2b0cb14ebd26e5629be1521858f66a7d4ae Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1609434 Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Tatiana Bradley <tatianabradley@google.com> Reviewed-by: Roland Shoemaker <bracewell@google.com> TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/446916 Reviewed-by: Tatiana Bradley <tatiana@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Heschi Kreinick <heschi@google.com>
This commit is contained in:
parent
ad5d2f64fb
commit
61ae0a37a8
@ -16,6 +16,7 @@ func TestDedupEnv(t *testing.T) {
|
||||
noCase bool
|
||||
in []string
|
||||
want []string
|
||||
wantErr bool
|
||||
}{
|
||||
{
|
||||
noCase: true,
|
||||
@ -43,11 +44,17 @@ func TestDedupEnv(t *testing.T) {
|
||||
in: []string{"dodgy", "entries"},
|
||||
want: []string{"dodgy", "entries"},
|
||||
},
|
||||
{
|
||||
// Filter out entries containing NULs.
|
||||
in: []string{"A=a\x00b", "B=b", "C\x00C=c"},
|
||||
want: []string{"B=b"},
|
||||
wantErr: true,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
got := dedupEnvCase(tt.noCase, tt.in)
|
||||
if !reflect.DeepEqual(got, tt.want) {
|
||||
t.Errorf("Dedup(%v, %q) = %q; want %q", tt.noCase, tt.in, got, tt.want)
|
||||
got, err := dedupEnvCase(tt.noCase, 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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -1190,7 +1190,11 @@ func (c *Cmd) environ() ([]string, error) {
|
||||
}
|
||||
}
|
||||
|
||||
return addCriticalEnv(dedupEnv(env)), err
|
||||
env, dedupErr := dedupEnv(env)
|
||||
if err == nil {
|
||||
err = dedupErr
|
||||
}
|
||||
return addCriticalEnv(env), err
|
||||
}
|
||||
|
||||
// Environ returns a copy of the environment in which the command would be run
|
||||
@ -1204,20 +1208,27 @@ 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.
|
||||
func dedupEnv(env []string) []string {
|
||||
// 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)
|
||||
}
|
||||
|
||||
// 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 {
|
||||
func dedupEnvCase(caseInsensitive bool, env []string) ([]string, error) {
|
||||
// Construct the output in reverse order, to preserve the
|
||||
// last occurrence of each key.
|
||||
var err error
|
||||
out := make([]string, 0, len(env))
|
||||
saw := make(map[string]bool, len(env))
|
||||
for n := len(env); n > 0; n-- {
|
||||
kv := env[n-1]
|
||||
|
||||
if 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.
|
||||
@ -1252,7 +1263,7 @@ func dedupEnvCase(caseInsensitive bool, env []string) []string {
|
||||
out[i], out[j] = out[j], out[i]
|
||||
}
|
||||
|
||||
return out
|
||||
return out, err
|
||||
}
|
||||
|
||||
// addCriticalEnv adds any critical environment variables that are required
|
||||
|
@ -1027,6 +1027,15 @@ func TestDedupEnvEcho(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestEnvNULCharacter(t *testing.T) {
|
||||
cmd := helperCommand(t, "echoenv", "FOO", "BAR")
|
||||
cmd.Env = append(cmd.Environ(), "FOO=foo\x00BAR=bar")
|
||||
out, err := cmd.CombinedOutput()
|
||||
if err == nil {
|
||||
t.Errorf("output = %q; want error", string(out))
|
||||
}
|
||||
}
|
||||
|
||||
func TestString(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
@ -7,6 +7,7 @@
|
||||
package syscall
|
||||
|
||||
import (
|
||||
"internal/bytealg"
|
||||
"runtime"
|
||||
"sync"
|
||||
"unicode/utf16"
|
||||
@ -115,12 +116,16 @@ func makeCmdLine(args []string) string {
|
||||
// the representation required by CreateProcess: a sequence of NUL
|
||||
// terminated strings followed by a nil.
|
||||
// Last bytes are two UCS-2 NULs, or four NUL bytes.
|
||||
func createEnvBlock(envv []string) *uint16 {
|
||||
// If any string contains a NUL, it returns (nil, EINVAL).
|
||||
func createEnvBlock(envv []string) (*uint16, error) {
|
||||
if len(envv) == 0 {
|
||||
return &utf16.Encode([]rune("\x00\x00"))[0]
|
||||
return &utf16.Encode([]rune("\x00\x00"))[0], nil
|
||||
}
|
||||
length := 0
|
||||
for _, s := range envv {
|
||||
if bytealg.IndexByteString(s, 0) != -1 {
|
||||
return nil, EINVAL
|
||||
}
|
||||
length += len(s) + 1
|
||||
}
|
||||
length += 1
|
||||
@ -135,7 +140,7 @@ func createEnvBlock(envv []string) *uint16 {
|
||||
}
|
||||
copy(b[i:i+1], []byte{0})
|
||||
|
||||
return &utf16.Encode([]rune(string(b)))[0]
|
||||
return &utf16.Encode([]rune(string(b)))[0], nil
|
||||
}
|
||||
|
||||
func CloseOnExec(fd Handle) {
|
||||
@ -400,12 +405,17 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle
|
||||
}
|
||||
}
|
||||
|
||||
envBlock, err := createEnvBlock(attr.Env)
|
||||
if err != nil {
|
||||
return 0, 0, err
|
||||
}
|
||||
|
||||
pi := new(ProcessInformation)
|
||||
flags := sys.CreationFlags | CREATE_UNICODE_ENVIRONMENT | _EXTENDED_STARTUPINFO_PRESENT
|
||||
if sys.Token != 0 {
|
||||
err = CreateProcessAsUser(sys.Token, argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, willInheritHandles, flags, createEnvBlock(attr.Env), dirp, &si.StartupInfo, pi)
|
||||
err = CreateProcessAsUser(sys.Token, argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, willInheritHandles, flags, envBlock, dirp, &si.StartupInfo, pi)
|
||||
} else {
|
||||
err = CreateProcess(argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, willInheritHandles, flags, createEnvBlock(attr.Env), dirp, &si.StartupInfo, pi)
|
||||
err = CreateProcess(argv0p, argvp, sys.ProcessAttributes, sys.ThreadAttributes, willInheritHandles, flags, envBlock, dirp, &si.StartupInfo, pi)
|
||||
}
|
||||
if err != nil {
|
||||
return 0, 0, err
|
||||
|
Loading…
Reference in New Issue
Block a user