From 78ef4abeec2c3931fd6d554f2f004096c2ccdcfe Mon Sep 17 00:00:00 2001 From: Nizamudeen Date: Tue, 7 Jan 2020 19:17:35 +0530 Subject: [PATCH] ceph: Fixing the potential file inclusion error This commit is also part of gosec error handling which handles the following issue: Fixed G304: Potential File inclusion by cleaning up the file path Signed-off-by: Nizamudeen --- pkg/daemon/ceph/agent/flexvolume/server.go | 5 +++-- pkg/daemon/ceph/osd/daemon.go | 2 +- pkg/daemon/ceph/util/util.go | 4 ++-- pkg/daemon/util/copybins.go | 5 +++-- pkg/operator/ceph/cluster/mon/config.go | 3 ++- pkg/operator/ceph/csi/util.go | 3 ++- pkg/operator/k8sutil/prometheus.go | 5 +++-- pkg/util/file.go | 2 +- 8 files changed, 17 insertions(+), 12 deletions(-) diff --git a/pkg/daemon/ceph/agent/flexvolume/server.go b/pkg/daemon/ceph/agent/flexvolume/server.go index 9b6aa4b06..2a0638da8 100644 --- a/pkg/daemon/ceph/agent/flexvolume/server.go +++ b/pkg/daemon/ceph/agent/flexvolume/server.go @@ -27,6 +27,7 @@ import ( "os" "os/exec" "path" + "path/filepath" "regexp" "strconv" "strings" @@ -173,7 +174,7 @@ func LoadFlexSettings(directory string) []byte { // Load the settings from the expected config file, ensure they are valid settings, then return them in // a json string to the caller var status flexvolume.DriverStatus - if output, err := ioutil.ReadFile(path.Join(directory, settingsFilename)); err == nil { + if output, err := ioutil.ReadFile(filepath.Clean(path.Join(directory, settingsFilename))); err == nil { if err := json.Unmarshal(output, &status); err == nil { if output, err = json.Marshal(status); err == nil { return output @@ -246,7 +247,7 @@ func configureFlexVolume(driverFile, driverDir, driverName string) error { } func copyFile(src, dest string) error { - srcFile, err := os.Open(src) + srcFile, err := os.Open(filepath.Clean(src)) if err != nil { return errors.Wrapf(err, "error opening source file %s", src) } diff --git a/pkg/daemon/ceph/osd/daemon.go b/pkg/daemon/ceph/osd/daemon.go index d30af6ffe..b6cdcb4bb 100644 --- a/pkg/daemon/ceph/osd/daemon.go +++ b/pkg/daemon/ceph/osd/daemon.go @@ -158,7 +158,7 @@ func Provision(context *clusterd.Context, agent *OsdAgent, crushLocation string) if err != nil { return errors.Wrapf(err, "failed to write connection config") } - src, err := ioutil.ReadFile(confFilePath) + src, err := ioutil.ReadFile(filepath.Clean(confFilePath)) if err != nil { return errors.Wrapf(err, "failed to copy connection config to /etc/ceph. failed to read the connection config") } diff --git a/pkg/daemon/ceph/util/util.go b/pkg/daemon/ceph/util/util.go index 4d617550f..db5b3ba1d 100644 --- a/pkg/daemon/ceph/util/util.go +++ b/pkg/daemon/ceph/util/util.go @@ -51,10 +51,10 @@ func FindRBDMappedFile(imageName, poolName, sysBusDir string) (string, error) { } for _, idFile := range files { - nameContent, err := ioutil.ReadFile(filepath.Join(sysBusDeviceDir, idFile.Name(), "name")) + nameContent, err := ioutil.ReadFile(filepath.Clean(filepath.Join(sysBusDeviceDir, idFile.Name(), "name"))) if err == nil && imageName == strings.TrimSpace(string(nameContent)) { // the image for the current rbd device matches, now try to match pool - poolContent, err := ioutil.ReadFile(filepath.Join(sysBusDeviceDir, idFile.Name(), "pool")) + poolContent, err := ioutil.ReadFile(filepath.Clean(filepath.Join(sysBusDeviceDir, idFile.Name(), "pool"))) if err == nil && poolName == strings.TrimSpace(string(poolContent)) { // match current device matches both image name and pool name, return the device return idFile.Name(), nil diff --git a/pkg/daemon/util/copybins.go b/pkg/daemon/util/copybins.go index c7012cc15..0ba343bba 100644 --- a/pkg/daemon/util/copybins.go +++ b/pkg/daemon/util/copybins.go @@ -21,6 +21,7 @@ import ( "os" "os/exec" "path" + "path/filepath" ) // override these for unit testing @@ -52,13 +53,13 @@ func copyBinary(sourceDir, targetDir, filename string) error { return nil } - sourceFile, err := os.Open(sourcePath) + sourceFile, err := os.Open(filepath.Clean(sourcePath)) if err != nil { return err } defer sourceFile.Close() - destinationFile, err := os.Create(targetPath) + destinationFile, err := os.Create(filepath.Clean(targetPath)) if err != nil { return err } diff --git a/pkg/operator/ceph/cluster/mon/config.go b/pkg/operator/ceph/cluster/mon/config.go index a1eb43574..a64144c80 100644 --- a/pkg/operator/ceph/cluster/mon/config.go +++ b/pkg/operator/ceph/cluster/mon/config.go @@ -22,6 +22,7 @@ import ( "io/ioutil" "os" "path" + "path/filepath" "strconv" "strings" @@ -253,7 +254,7 @@ func genSecret(executor exec.Executor, configDir, name string, args []string) (s return "", errors.Wrapf(err, "failed to gen secret") } - contents, err := ioutil.ReadFile(path) + contents, err := ioutil.ReadFile(filepath.Clean(path)) if err != nil { return "", errors.Wrapf(err, "failed to read secret file") } diff --git a/pkg/operator/ceph/csi/util.go b/pkg/operator/ceph/csi/util.go index 422c44c72..705415bd6 100644 --- a/pkg/operator/ceph/csi/util.go +++ b/pkg/operator/ceph/csi/util.go @@ -20,6 +20,7 @@ import ( "bytes" "io/ioutil" "os" + "path/filepath" "strconv" "strings" "text/template" @@ -33,7 +34,7 @@ import ( ) func loadTemplate(name, templatePath string, p templateParam) (string, error) { - b, err := ioutil.ReadFile(templatePath) + b, err := ioutil.ReadFile(filepath.Clean(templatePath)) if err != nil { return "", err } diff --git a/pkg/operator/k8sutil/prometheus.go b/pkg/operator/k8sutil/prometheus.go index 13373b089..e37b01788 100644 --- a/pkg/operator/k8sutil/prometheus.go +++ b/pkg/operator/k8sutil/prometheus.go @@ -21,6 +21,7 @@ import ( "bytes" "fmt" "io/ioutil" + "path/filepath" monitoringv1 "github.com/coreos/prometheus-operator/pkg/apis/monitoring/v1" monitoringclient "github.com/coreos/prometheus-operator/pkg/client/versioned" @@ -41,7 +42,7 @@ func getMonitoringClient() (*monitoringclient.Clientset, error) { // GetServiceMonitor returns servicemonitor or an error func GetServiceMonitor(filePath string) (*monitoringv1.ServiceMonitor, error) { - file, err := ioutil.ReadFile(filePath) + file, err := ioutil.ReadFile(filepath.Clean(filePath)) if err != nil { return nil, fmt.Errorf("servicemonitor file could not be fetched. %v", err) } @@ -78,7 +79,7 @@ func CreateOrUpdateServiceMonitor(serviceMonitorDefinition *monitoringv1.Service // GetPrometheusRule returns provided prometheus rules or an error func GetPrometheusRule(ruleFilePath string) (*monitoringv1.PrometheusRule, error) { - ruleFile, err := ioutil.ReadFile(ruleFilePath) + ruleFile, err := ioutil.ReadFile(filepath.Clean(ruleFilePath)) if err != nil { return nil, fmt.Errorf("prometheusRules file could not be fetched. %v", err) } diff --git a/pkg/util/file.go b/pkg/util/file.go index ae25abaf9..eb3461a95 100644 --- a/pkg/util/file.go +++ b/pkg/util/file.go @@ -40,7 +40,7 @@ func WriteFile(filePath string, contentBuffer bytes.Buffer) error { } func WriteFileToLog(logger *capnslog.PackageLogger, path string) { - contents, err := ioutil.ReadFile(path) + contents, err := ioutil.ReadFile(filepath.Clean(path)) if err != nil { logger.Warningf("failed to write file %s to log: %+v", path, err) return