From cfa27fd672baa408987a7bed4233c471be1ea3df Mon Sep 17 00:00:00 2001 From: hanchao Date: Fri, 11 Nov 2022 00:35:53 +0800 Subject: [PATCH] golang: fix CVE-2022-41716 Score: 7.5 Reference: https://go-review.googlesource.com/c/go/+/446916 Conflict: src/os/exec/exec.go;src/syscall/exec_windows.go Reason: fix CVE-2022-41716 (cherry picked from commit 76ac33e67eb0a5b5dcde7bce38edda989149c158) --- ...reject-environment-variables-contain.patch | 248 ++++++++++++++++++ golang.spec | 17 +- 2 files changed, 260 insertions(+), 5 deletions(-) create mode 100644 0023-syscall-os-exec-reject-environment-variables-contain.patch diff --git a/0023-syscall-os-exec-reject-environment-variables-contain.patch b/0023-syscall-os-exec-reject-environment-variables-contain.patch new file mode 100644 index 0000000..50c3d75 --- /dev/null +++ b/0023-syscall-os-exec-reject-environment-variables-contain.patch @@ -0,0 +1,248 @@ +From 0c539fa7a4a9d29252523e41e073198195ba6691 Mon Sep 17 00:00:00 2001 +From: Damien Neil +Date: Mon, 17 Oct 2022 17:38:29 -0700 +Subject: [PATCH] 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 +Reviewed-by: Tatiana Bradley +Reviewed-by: Roland Shoemaker +TryBot-Result: Security TryBots +Reviewed-on: https://go-review.googlesource.com/c/go/+/446916 +Reviewed-by: Tatiana Bradley +TryBot-Result: Gopher Robot +Run-TryBot: Matthew Dempsky +Reviewed-by: Heschi Kreinick + +Reference: https://go-review.googlesource.com/c/go/+/446916 +Conflict: src/os/exec/exec.go;src/syscall/exec_windows.go +--- + src/os/exec/env_test.go | 19 +++++++++++++------ + src/os/exec/exec.go | 38 ++++++++++++++++++++++++++++++++----- + src/os/exec/exec_test.go | 9 +++++++++ + src/syscall/exec_windows.go | 20 ++++++++++++++----- + 4 files changed, 70 insertions(+), 16 deletions(-) + +diff --git a/src/os/exec/env_test.go b/src/os/exec/env_test.go +index b5ac398..47b7c04 100644 +--- a/src/os/exec/env_test.go ++++ b/src/os/exec/env_test.go +@@ -11,9 +11,10 @@ import ( + + func TestDedupEnv(t *testing.T) { + tests := []struct { +- noCase bool +- in []string +- want []string ++ noCase bool ++ in []string ++ want []string ++ wantErr bool + }{ + { + noCase: true, +@@ -29,11 +30,17 @@ func TestDedupEnv(t *testing.T) { + in: []string{"=a", "=b", "foo", "bar"}, + want: []string{"=b", "foo", "bar"}, + }, ++ { ++ // 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) + } + } + } +diff --git a/src/os/exec/exec.go b/src/os/exec/exec.go +index 0c49575..6f5c61b 100644 +--- a/src/os/exec/exec.go ++++ b/src/os/exec/exec.go +@@ -414,7 +414,7 @@ func (c *Cmd) Start() error { + } + c.childFiles = append(c.childFiles, c.ExtraFiles...) + +- envv, err := c.envv() ++ env, err := c.environ() + if err != nil { + return err + } +@@ -422,7 +422,7 @@ func (c *Cmd) Start() error { + c.Process, err = os.StartProcess(c.Path, c.argv(), &os.ProcAttr{ + Dir: c.Dir, + Files: c.childFiles, +- Env: addCriticalEnv(dedupEnv(envv)), ++ Env: env, + Sys: c.SysProcAttr, + }) + if err != nil { +@@ -738,16 +738,21 @@ func minInt(a, b int) int { + // 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 { ++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) { ++ var err error + out := make([]string, 0, len(env)) + saw := make(map[string]int, len(env)) // key => index into out + for _, kv := range env { ++ if strings.IndexByte(kv, 0) != -1 { ++ err = errors.New("exec: environment variable contains NUL") ++ continue ++ } + eq := strings.Index(kv, "=") + if eq < 0 { + out = append(out, kv) +@@ -764,7 +769,7 @@ func dedupEnvCase(caseInsensitive bool, env []string) []string { + saw[k] = len(out) + out = append(out, kv) + } +- return out ++ return out, err + } + + // addCriticalEnv adds any critical environment variables that are required +@@ -787,3 +792,26 @@ func addCriticalEnv(env []string) []string { + } + return append(env, "SYSTEMROOT="+os.Getenv("SYSTEMROOT")) + } ++ ++// environ returns a best-effort copy of the environment in which the command ++// would be run as it is currently configured. If an error occurs in computing ++// the environment, it is returned alongside the best-effort copy. ++func (c *Cmd) environ() ([]string, error) { ++ env, err := c.envv() ++ if err != nil { ++ return 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 ++// as it is currently configured. ++func (c *Cmd) Environ() []string { ++ // Intentionally ignore errors: environ returns a best-effort environment no matter what. ++ env, _ := c.environ() ++ return env ++} +diff --git a/src/os/exec/exec_test.go b/src/os/exec/exec_test.go +index d854e0d..d03eab2 100644 +--- a/src/os/exec/exec_test.go ++++ b/src/os/exec/exec_test.go +@@ -1104,6 +1104,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) { + echoPath, err := exec.LookPath("echo") + if err != nil { +diff --git a/src/syscall/exec_windows.go b/src/syscall/exec_windows.go +index 9d10d6a..50892be 100644 +--- a/src/syscall/exec_windows.go ++++ b/src/syscall/exec_windows.go +@@ -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 +-- +2.33.0 + diff --git a/golang.spec b/golang.spec index 6123a94..3e881f4 100644 --- a/golang.spec +++ b/golang.spec @@ -62,7 +62,7 @@ Name: golang Version: 1.17.3 -Release: 10 +Release: 11 Summary: The Go Programming Language License: BSD and Public Domain URL: https://golang.org/ @@ -149,10 +149,10 @@ Obsoletes: %{name}-vim < 1.4 Obsoletes: emacs-%{name} < 1.4 Requires: openEuler-rpm-config -Patch6001: 0001-release-branch.go1.17-crypto-elliptic-tolerate-zero-.patch -Patch6002: 0002-release-branch.go1.17-encoding-pem-fix-stack-overflo.patch -Patch6003: 0003-release-branch.go1.17-syscall-fix-ForkLock-spurious-.patch -Patch6004: 0004-backport-cmd-link-mark-unexported-methods-for-plugins.patch +Patch6001: 0001-release-branch.go1.17-crypto-elliptic-tolerate-zero-.patch +Patch6002: 0002-release-branch.go1.17-encoding-pem-fix-stack-overflo.patch +Patch6003: 0003-release-branch.go1.17-syscall-fix-ForkLock-spurious-.patch +Patch6004: 0004-backport-cmd-link-mark-unexported-methods-for-plugins.patch Patch6005: 0005-release-branch.go1.17-net-http-preserve-nil-values-i.patch Patch6006: 0006-release-branch.go1.17-go-parser-limit-recursion-dept.patch Patch6007: 0007-release-branch.go1.17-net-http-don-t-strip-whitespac.patch @@ -171,6 +171,7 @@ Patch6019: 0019-release-branch.go1.18-net-http-update-bundled-golang.patch Patch6020: 0020-release-branch.go1.18-regexp-limit-size-of-parsed-re.patch Patch6021: 0021-release-branch.go1.18-net-http-httputil-avoid-query-.patch Patch6022: 0022-release-branch.go1.18-archive-tar-limit-size-of-head.patch +Patch6023: 0023-syscall-os-exec-reject-environment-variables-contain.patch ExclusiveArch: %{golang_arches} @@ -409,6 +410,12 @@ fi %files devel -f go-tests.list -f go-misc.list -f go-src.list %changelog +* Fri Oct 11 2022 hanchao - 1.17.3-11 +- Type:CVE +- CVE:CVE-2022-41716 +- SUG:NA +- DESC: fix CVE-2022-41716 + * Mon Oct 10 2022 hanchao - 1.17.3-10 - Type:CVE - CVE:CVE-2022-41715,CVE-2022-2880,CVE-2022-2879 -- Gitee