[release-branch.go1.25] cmd/go: fix pkg-config flag sanitization

Implement a new pkg-config safe flag list (containing everything except
for --log-file) and use that when checking flags passed to pkg-config,
instead of using checkCompilerFlags.

Updates #77387
Fixes #77438

Change-Id: Id6141d0a2934053aa43e3aa8ce402bd499c4c028
Reviewed-on: https://go-review.googlesource.com/c/go/+/741042
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 28fbdf7acb4146b5bc3d88128e407d1344691839)
Reviewed-on: https://go-review.googlesource.com/c/go/+/745481
Reviewed-by: David Chase <drchase@google.com>

diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index 9d4429a51c2cb7aa8f890b5281a65ab6b8088b8a..6bfc83aae262116d2449f24af030654d77543b99 100644 (file)
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -1637,10 +1637,7 @@ func (b *Builder) getPkgConfigFlags(a *Action) (cflags, ldflags []string, err er
                        }
                }
 
-               // Running 'pkg-config' can cause execution of
-               // arbitrary code using flags that are not in
-               // the safelist.
-               if err := checkCompilerFlags("CFLAGS", "pkg-config --cflags", pcflags); err != nil {
+               if err := checkPkgConfigFlags("", "pkg-config", pcflags); err != nil {
                        return nil, nil, err
                }
 

diff --git a/src/cmd/go/internal/work/security.go b/src/cmd/go/internal/work/security.go
index 68d2706051b08bc9654315cfc5d320e8aac2e6b0..f80b2bcb2b64cf509f2d778df99c173e2c17f699 100644 (file)
--- a/src/cmd/go/internal/work/security.go
+++ b/src/cmd/go/internal/work/security.go
@@ -254,6 +254,58 @@ var validLinkerFlagsWithNextArg = []string{
        "-Wl,-undefined",
 }
 
+var validPkgConfigFlags = []*lazyregexp.Regexp{
+       re(`--atleast-pkgconfig-version=\d+\.\d+\.\d+`),
+       re(`--atleast-version=\d+\.\d+\.\d+`),
+       re(`--cflags-only-I`),
+       re(`--cflags`),
+       re(`--define-prefix`),
+       re(`--define-variable=[A-Za-z_][A-Za-z0-9_]*=[^@\-]*`),
+       re(`--digraph`),
+       re(`--dont-define-prefix`),
+       re(`--dont-relocate-paths`),
+       re(`--dump-personality`),
+       re(`--env-only`),
+       re(`--errors-to-stdout`),
+       re(`--exact-version=\d+\.\d+\.\d+`),
+       re(`--exists`),
+       re(`--fragment-filter=[A-Za-z_][a-zA-Z0-9_]*`),
+       re(`--ignore-conflicts`),
+       re(`--internal-cflags`),
+       re(`--keep-system-cflags`),
+       re(`--keep-system-libs`),
+       re(`--libs-only-l`),
+       re(`--libs-only-L`),
+       re(`--libs`),
+       re(`--list-all`),
+       re(`--list-package-names`),
+       re(`--max-version=\d+\.\d+\.\d+`),
+       re(`--maximum-traverse-depth=[0-9]+`),
+       re(`--modversion`),
+       re(`--msvc-syntax`),
+       re(`--no-cache`),
+       re(`--no-provides`),
+       re(`--no-uninstalled`),
+       re(`--path`),
+       re(`--personality=(triplet|filename)`),
+       re(`--prefix-variable=[A-Za-z_][a-zA-Z0-9_]*`),
+       re(`--print-errors`),
+       re(`--print-provides`),
+       re(`--print-requires-private`),
+       re(`--print-requires`),
+       re(`--print-variables`),
+       re(`--pure`),
+       re(`--shared`),
+       re(`--short-errors`),
+       re(`--silence-errors`),
+       re(`--simulate`),
+       re(`--static`),
+       re(`--uninstalled`),
+       re(`--validate`),
+       re(`--variable=[A-Za-z_][a-zA-Z0-9_]*`),
+       re(`--with-path=[^@\-].*`),
+}
+
 func checkCompilerFlags(name, source string, list []string) error {
        checkOverrides := true
        return checkFlags(name, source, list, nil, validCompilerFlags, validCompilerFlagsWithNextArg, checkOverrides)
@@ -264,6 +316,11 @@ func checkLinkerFlags(name, source string, list []string) error {
        return checkFlags(name, source, list, invalidLinkerFlags, validLinkerFlags, validLinkerFlagsWithNextArg, checkOverrides)
 }
 
+func checkPkgConfigFlags(name, source string, list []string) error {
+       checkOverrides := false
+       return checkFlags(name, source, list, nil, validPkgConfigFlags, nil, checkOverrides)
+}
+
 // checkCompilerFlagsForInternalLink returns an error if 'list'
 // contains a flag or flags that may not be fully supported by
 // internal linking (meaning that we should punt the link to the

diff --git a/src/cmd/go/testdata/script/cgo_bad_directives.txt b/src/cmd/go/testdata/script/cgo_bad_directives.txt
index 7d28171fad47e708df5a9dc9445ad5469bfddb85..0c64b6d9a40f1693c9ec424130d270d2b8fca226 100644 (file)
--- a/src/cmd/go/testdata/script/cgo_bad_directives.txt
+++ b/src/cmd/go/testdata/script/cgo_bad_directives.txt
@@ -45,6 +45,11 @@ cp y_pkgconfig_at_foo.txt y.go
 ! go build x
 stderr 'invalid pkg-config package name: @foo'
 
+# Reject #cgo pkg-config: --log-file=/tmp/log
+cp y_pkgconfig_log_file.txt y.go
+! go build x
+stderr 'invalid flag in pkg-config: --log-file=/tmp/log'
+
 # Reject #cgo CFLAGS: @foo
 cp y_cflags_at_foo.txt y.go
 ! go build x
@@ -108,6 +113,10 @@ import "C"
 package x
 // #cgo pkg-config: @foo
 import "C"
+-- y_pkgconfig_log_file.txt --
+package x
+// #cgo pkg-config: --log-file=/tmp/log
+import "C"
 -- y_cflags_at_foo.txt --
 package x
 // #cgo CFLAGS: @foo