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.

Fixes #77387

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>

diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index d1c0c4ea6361259a9c7226d176994b01fbf5d99e..13962fd1eaa69fe459fbf6e46bd0e1f82d35b6be 100644 (file)
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -1797,10 +1797,7 @@ func (b *Builder) getPkgConfigFlags(a *Action, p *load.Package) (cflags, ldflags
                        }
                }
 
-               // 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 80b3f8797cc3cee1574931394e11e2ccd98cf05d..b5acb0a0ee09a6ed3de0086b8eff5c230c05105a 100644 (file)
--- a/src/cmd/go/internal/work/security.go
+++ b/src/cmd/go/internal/work/security.go
@@ -253,6 +253,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)
@@ -263,6 +315,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