From ab8822963bc7bf3ff0e1dd89296dbf6c007d85d5 Mon Sep 17 00:00:00 2001 From: Bricktop Date: Thu, 14 Oct 2021 00:05:13 +0200 Subject: [PATCH] Add exclusions to make gosec happy (#2540) * Add exclusions to make gosec happy Signed-off-by: Marcel Mueller * Add forgotten file Signed-off-by: Marcel Mueller --- cmd/kyverno/main.go | 4 +++- pkg/kyverno/apply/apply_command.go | 6 ++++-- pkg/kyverno/common/common.go | 24 +++++++++++++++++------- pkg/kyverno/common/fetch.go | 6 ++++-- pkg/kyverno/test/test_command.go | 15 ++++++++------- pkg/testrunner/scenario.go | 3 ++- pkg/testrunner/utils.go | 3 ++- pkg/webhookconfig/common.go | 3 ++- 8 files changed, 42 insertions(+), 22 deletions(-) diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index 4b4fb8a712..15706958cd 100755 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -5,7 +5,9 @@ import ( "flag" "fmt" "net/http" - _ "net/http/pprof" + + // We currently accept the risk of exposing pprof and rely on users to protect the endpoint. + _ "net/http/pprof" // #nosec "os" "strings" "time" diff --git a/pkg/kyverno/apply/apply_command.go b/pkg/kyverno/apply/apply_command.go index 13940d7dce..c122a6a9c6 100644 --- a/pkg/kyverno/apply/apply_command.go +++ b/pkg/kyverno/apply/apply_command.go @@ -205,7 +205,8 @@ func applyCommandHelper(resourcePaths []string, cluster bool, policyReport bool, // the truncation of files for the case when mutateLogPath is dir, is handled under pkg/kyverno/apply/common.go if !mutateLogPathIsDir && mutateLogPath != "" { mutateLogPath = filepath.Clean(mutateLogPath) - _, err := os.OpenFile(mutateLogPath, os.O_TRUNC|os.O_WRONLY, 0600) + // Necessary for us to include the file via variable as it is part of the CLI. + _, err := os.OpenFile(mutateLogPath, os.O_TRUNC|os.O_WRONLY, 0600) // #nosec G304 if err != nil { if !sanitizederror.IsErrorSanitized(err) { @@ -377,7 +378,8 @@ func createFileOrFolder(mutateLogPath string, mutateLogPathIsDir bool) error { } mutateLogPath = filepath.Clean(mutateLogPath) - file, err := os.OpenFile(mutateLogPath, os.O_RDONLY|os.O_CREATE, 0600) + // Necessary for us to create the file via variable as it is part of the CLI. + file, err := os.OpenFile(mutateLogPath, os.O_RDONLY|os.O_CREATE, 0600) // #nosec G304 if err != nil { return sanitizederror.NewWithError(fmt.Sprintf("failed to create file"), err) diff --git a/pkg/kyverno/common/common.go b/pkg/kyverno/common/common.go index 75628dd36b..7edbdb1226 100644 --- a/pkg/kyverno/common/common.go +++ b/pkg/kyverno/common/common.go @@ -118,7 +118,8 @@ func GetPolicies(paths []string) (policies []*v1.ClusterPolicy, errors []error) } else { var fileBytes []byte if isHttpPath { - resp, err := http.Get(path) + // We accept here that a random URL might be called based on user provided input. + resp, err := http.Get(path) // #nosec if err != nil { err := fmt.Errorf("failed to process %v: %v", path, err.Error()) errors = append(errors, err) @@ -140,7 +141,8 @@ func GetPolicies(paths []string) (policies []*v1.ClusterPolicy, errors []error) } } else { path = filepath.Clean(path) - fileBytes, err = ioutil.ReadFile(path) + // We accept the risk of including a user provided file here. + fileBytes, err = ioutil.ReadFile(path) // #nosec G304 if err != nil { err := fmt.Errorf("failed to process %v: %v", path, err.Error()) errors = append(errors, err) @@ -344,7 +346,8 @@ func GetCRDs(paths []string) (unstructuredCrds []*unstructured.Unstructured, err func GetCRD(path string) (unstructuredCrds []*unstructured.Unstructured, err error) { path = filepath.Clean(path) unstructuredCrds = make([]*unstructured.Unstructured, 0) - yamlbytes, err := ioutil.ReadFile(path) + // We accept the risk of including a user provided file here. + yamlbytes, err := ioutil.ReadFile(path) // #nosec G304 if err != nil { return nil, err } @@ -426,8 +429,12 @@ func GetVariable(variablesString, valuesFile string, fs billy.Filesystem, isGit fmt.Printf("Unable to open variable file: %s. error: %s", valuesFile, err) } yamlFile, err = ioutil.ReadAll(filep) + if err != nil { + fmt.Printf("Unable to read variable files: %s. error: %s \n", filep, err) + } } else { - yamlFile, err = ioutil.ReadFile(filepath.Join(policyResourcePath, valuesFile)) + // We accept the risk of including a user provided file here. + yamlFile, err = ioutil.ReadFile(filepath.Join(policyResourcePath, valuesFile)) // #nosec G304 if err != nil { fmt.Printf("\n Unable to open variable file: %s. error: %s \n", valuesFile, err) } @@ -653,16 +660,19 @@ func PrintMutatedOutput(mutateLogPath string, mutateLogPathIsDir bool, yaml stri mutateLogPath = filepath.Clean(mutateLogPath) if !mutateLogPathIsDir { // truncation for the case when mutateLogPath is a file (not a directory) is handled under pkg/kyverno/apply/test_command.go - f, err = os.OpenFile(mutateLogPath, os.O_APPEND|os.O_WRONLY, 0600) + f, err = os.OpenFile(mutateLogPath, os.O_APPEND|os.O_WRONLY, 0600) // #nosec G304 } else { - f, err = os.OpenFile(mutateLogPath+"/"+fileName+".yaml", os.O_CREATE|os.O_WRONLY, 0600) + f, err = os.OpenFile(mutateLogPath+"/"+fileName+".yaml", os.O_CREATE|os.O_WRONLY, 0600) // #nosec G304 } if err != nil { return err } if _, err := f.Write([]byte(yaml)); err != nil { - f.Close() + closeErr := f.Close() + if closeErr != nil { + log.Log.Error(closeErr, "failed to close file") + } return err } if err := f.Close(); err != nil { diff --git a/pkg/kyverno/common/fetch.go b/pkg/kyverno/common/fetch.go index 0345accd46..1da5973c65 100644 --- a/pkg/kyverno/common/fetch.go +++ b/pkg/kyverno/common/fetch.go @@ -221,7 +221,8 @@ func getFileBytes(path string) ([]byte, error) { ) if IsHttpRegex.MatchString(path) { - resp, err := http.Get(path) + // We accept here that a random URL might be called based on user provided input. + resp, err := http.Get(path) // #nosec if err != nil { return nil, err } @@ -237,7 +238,8 @@ func getFileBytes(path string) ([]byte, error) { } } else { path = filepath.Clean(path) - file, err = ioutil.ReadFile(path) + // We accept the risk of including a user provided file here. + file, err = ioutil.ReadFile(path) // #nosec G304 if err != nil { return nil, err } diff --git a/pkg/kyverno/test/test_command.go b/pkg/kyverno/test/test_command.go index 59ca38bf5b..5d665583f6 100644 --- a/pkg/kyverno/test/test_command.go +++ b/pkg/kyverno/test/test_command.go @@ -39,7 +39,7 @@ import ( ) var longHelp = ` -Test command provides a facility to test policies on resources. User should provide the path of the folder containing test.yaml file. +Test command provides a facility to test policies on resources. User should provide the path of the folder containing test.yaml file. kyverno test or @@ -54,7 +54,7 @@ The test.yaml have 4 parts: var exampleHelp = ` test.yaml format: -For Validate Policy +For Validate Policy - name: test-1 policies: - @@ -67,7 +67,7 @@ For Validate Policy rule: resource: namespace: (OPTIONAL) - kind: + kind: result: @@ -86,7 +86,7 @@ For Mutate Policy rule: resource: namespace: (OPTIONAL) - kind: + kind: patchedResource: result: @@ -103,13 +103,13 @@ For Mutate Policy rule: resource: namespace: (OPTIONAL) - kind: + kind: patchedResource: result: Result description: pass --> patched Resource generated from engine equals to patched Resource provided by the user. -fail --> patched Resource generated from engine is not equals to patched provided by the user. +fail --> patched Resource generated from engine is not equals to patched provided by the user. skip --> rule is not applied. For more visit --> https://kyverno.io/docs/kyverno-cli/#test @@ -306,7 +306,8 @@ func getLocalDirTestFiles(fs billy.Filesystem, path, fileName, valuesFile string continue } if strings.Contains(file.Name(), fileName) { - yamlFile, err := ioutil.ReadFile(filepath.Join(path, file.Name())) + // We accept the risk of including files here as we read the test dir only. + yamlFile, err := ioutil.ReadFile(filepath.Join(path, file.Name())) // #nosec G304 if err != nil { errors = append(errors, sanitizederror.NewWithError("unable to read yaml", err)) continue diff --git a/pkg/testrunner/scenario.go b/pkg/testrunner/scenario.go index 262d7d5d9a..9bb7424cd4 100644 --- a/pkg/testrunner/scenario.go +++ b/pkg/testrunner/scenario.go @@ -119,7 +119,8 @@ func loadFile(t *testing.T, path string) ([]byte, error) { return nil, err } path = filepath.Clean(path) - return ioutil.ReadFile(path) + // We accept the risk of including a user provided file here. + return ioutil.ReadFile(path) // #nosec G304 } func runScenario(t *testing.T, s *Scenario) bool { diff --git a/pkg/testrunner/utils.go b/pkg/testrunner/utils.go index eaf1cde98b..a6824f2268 100644 --- a/pkg/testrunner/utils.go +++ b/pkg/testrunner/utils.go @@ -15,7 +15,8 @@ func LoadFile(path string) ([]byte, error) { return nil, err } path = filepath.Clean(path) - return ioutil.ReadFile(path) + // We accept the risk of including a user provided file here. + return ioutil.ReadFile(path) // #nosec G304 } var kindToResource = map[string]string{ diff --git a/pkg/webhookconfig/common.go b/pkg/webhookconfig/common.go index 8a0ae763be..0aca7e1399 100644 --- a/pkg/webhookconfig/common.go +++ b/pkg/webhookconfig/common.go @@ -44,7 +44,8 @@ func extractCA(config *rest.Config) (result []byte) { if fileName != "" { fileName = filepath.Clean(fileName) - result, err := ioutil.ReadFile(fileName) + // We accept the risk of including a user provided file here. + result, err := ioutil.ReadFile(fileName) // #nosec G304 if err != nil { return nil