From cc0f9968db1abe397e6c93bf4f5dff51be20f914 Mon Sep 17 00:00:00 2001 From: Etienne Perot Date: Fri, 20 Jan 2023 11:41:38 -0800 Subject: pkg/build/gvisor: make gVisor configuration parsing stricter (#3617) This enforces that the configuration string passed to gVisor is only made up of known flags. Prior to this change, it was possible to pass any arbitrary flags as configuration, which would be silently ignored. --- pkg/build/gvisor.go | 49 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/pkg/build/gvisor.go b/pkg/build/gvisor.go index 861fced1b..89b583db6 100644 --- a/pkg/build/gvisor.go +++ b/pkg/build/gvisor.go @@ -27,22 +27,24 @@ func (gvisor gvisor) build(params Params) (ImageDetails, error) { // Bring down bazel daemon right away. We don't need it running and consuming memory. defer osutil.RunCmd(10*time.Minute, params.KernelDir, params.Compiler, "shutdown") - config := strings.Fields(string(params.Config)) + config, err := parseGVisorConfig(params.Config) + if err != nil { + return ImageDetails{}, fmt.Errorf("cannot parse gVisor configuration: %w", err) + } args := []string{} target := "//runsc:runsc" - race := raceEnabled(config) - if race { + if config.Race { args = append(args, "--@io_bazel_rules_go//go/config:race") target = "//runsc:runsc-race" } - if coverageEnabled(config) { + if config.Coverage { coverageFiles := "//pkg/..." exclusions := []string{ "//pkg/sentry/platform", "//pkg/ring0", // Breaks kvm. "//pkg/coverage:coverage", // Too slow. } - if race { + if config.Race { // These targets use go:norace, which is not // respected by coverage instrumentation. Race builds // will be instrumented with atomic coverage (using @@ -97,20 +99,33 @@ func (gvisor) clean(kernelDir, targetArch string) error { return nil } -func coverageEnabled(config []string) bool { - for _, flag := range config { - if flag == "-cover" { - return true - } - } - return false +// Known gVisor configuration flags. +const ( + gvisorFlagCover = "-cover" + gvisorFlagRace = "-race" +) + +// gvisorConfig is a gVisor configuration. +type gvisorConfig struct { + // Coverage represents whether code coverage is enabled. + Coverage bool + + // Race represents whether race condition detection is enabled. + Race bool } -func raceEnabled(config []string) bool { - for _, flag := range config { - if flag == "-race" { - return true +// parseGVisorConfig parses a set of flags into a `gvisorConfig`. +func parseGVisorConfig(config []byte) (gvisorConfig, error) { + var cfg gvisorConfig + for _, flag := range strings.Fields(string(config)) { + switch flag { + case gvisorFlagCover: + cfg.Coverage = true + case gvisorFlagRace: + cfg.Race = true + default: + return cfg, fmt.Errorf("unknown gVisor configuration flag: %q", flag) } } - return false + return cfg, nil } -- cgit mrf-deployment