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>

1 files changed, 15 insertions, 4 deletions

diff --git a/src/os/exec/exec.go b/src/os/exec/exec.go
index 31395c13df..18f265ee4e 100644
--- a/src/os/exec/exec.go
+++ b/src/os/exec/exec.go
@@ -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