Skip to content

Commit

Permalink
Fix gosec issue of: Potential file inclusion via variable
Browse files Browse the repository at this point in the history
Signed-off-by: Ezra Silvera <[email protected]>
  • Loading branch information
ezrasilvera committed Nov 18, 2020
1 parent 5978d81 commit f68180d
Show file tree
Hide file tree
Showing 16 changed files with 23 additions and 8 deletions.
2 changes: 2 additions & 0 deletions pkg/certificates/bootstrap/cert-manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,12 @@ func (f *FileCertificateManager) rotateCerts() error {
}

func (f *FileCertificateManager) loadCertificates() (serverCrt *tls.Certificate, err error) {
// #nosec No risk for path injection. Used for specific cert file for key rotation
certBytes, err := ioutil.ReadFile(f.certBytesPath)
if err != nil {
return nil, err
}
// #nosec No risk for path injection. Used for specific cert file for key rotation
keyBytes, err := ioutil.ReadFile(f.keyBytesPath)
if err != nil {
return nil, err
Expand Down
2 changes: 2 additions & 0 deletions pkg/certificates/triple/cert/cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,10 @@ func GenerateSelfSignedCertKeyWithFixtures(host string, alternateIPs []net.IP, a
certFixturePath := path.Join(fixtureDirectory, baseName+".crt")
keyFixturePath := path.Join(fixtureDirectory, baseName+".key")
if len(fixtureDirectory) > 0 {
// #nosec No risk for path injection. Used internally to read cert files
cert, err := ioutil.ReadFile(certFixturePath)
if err == nil {
// #nosec No risk for path injection. Used internally to read cert files
key, err := ioutil.ReadFile(keyFixturePath)
if err == nil {
return cert, key, nil
Expand Down
1 change: 1 addition & 0 deletions pkg/cloud-init/cloud-init.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ func findCloudInitConfigDriveSecretVolume(volumes []v1.Volume) *v1.Volume {

func readFileFromDir(basedir, secretFile string) (string, error) {
userDataSecretFile := filepath.Join(basedir, secretFile)
// #nosec No risk for path injection: basedir & secretFile are static strings
userDataSecret, err := ioutil.ReadFile(userDataSecretFile)
if err != nil {
log.Log.V(2).Reason(err).
Expand Down
1 change: 1 addition & 0 deletions pkg/ephemeral-disk-utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ func FileExists(path string) (bool, error) {
}
func Md5CheckSum(filePath string) ([]byte, error) {

// #nosec no risk for path injection: only used to calculate MD5 of files
file, err := os.Open(filePath)
if err != nil {
return nil, err
Expand Down
1 change: 1 addition & 0 deletions pkg/util/net/ip/ip.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ func NormalizeIPAddress(ipAddress string) string {
}

func isIPv6Disabled(filename string) bool {
// #nosec No injection path risk: filename is set to the const `disableIPv6Path`
data, err := ioutil.ReadFile(filename)
if err != nil {
return true
Expand Down
2 changes: 1 addition & 1 deletion pkg/virt-handler/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func InitializeGhostRecordCache(directoryPath string) error {
continue
}
recordPath := filepath.Join(ghostRecordDir, file.Name())

// #nosec no risk for path injection. Used only for testing and using static location
fileBytes, err := ioutil.ReadFile(recordPath)
if err != nil {
log.Log.Reason(err).Errorf("Unable to read ghost record file at path %s", recordPath)
Expand Down
3 changes: 2 additions & 1 deletion pkg/virt-handler/container-disk/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func (m *mounter) getMountTargetRecord(vmi *v1.VirtualMachineInstance) (*vmiMoun
}

// if not there, see if record is on disk, this can happen if virt-handler restarts
recordFile := filepath.Join(m.mountStateDir, string(vmi.UID))
recordFile := filepath.Join(m.mountStateDir, filepath.Clean(string(vmi.UID)))

exists, err := diskutils.FileExists(recordFile)
if err != nil {
Expand All @@ -118,6 +118,7 @@ func (m *mounter) getMountTargetRecord(vmi *v1.VirtualMachineInstance) (*vmiMoun

if exists {
record := vmiMountTargetRecord{}
// #nosec No risk for path injection. Using static base and cleaned filename
bytes, err := ioutil.ReadFile(recordFile)
if err != nil {
return nil, err
Expand Down
2 changes: 2 additions & 0 deletions pkg/virt-handler/device-manager/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ func (h *DeviceUtilsHandler) GetDeviceDriver(basepath string, pciAddress string)
func (h *DeviceUtilsHandler) GetDeviceNumaNode(basepath string, pciAddress string) (numaNode int) {
numaNode = -1
numaNodePath := filepath.Join(basepath, pciAddress, "numa_node")
// #nosec No risk for path injection. Reading static path of NUMA node info
numaNodeStr, err := ioutil.ReadFile(numaNodePath)
if err != nil {
log.DefaultLogger().Reason(err).Errorf("failed to read numa_node %s for device %s", numaNodePath, pciAddress)
Expand All @@ -89,6 +90,7 @@ func (h *DeviceUtilsHandler) GetDeviceNumaNode(basepath string, pciAddress strin
}

func (h *DeviceUtilsHandler) GetDevicePCIID(basepath string, pciAddress string) (string, error) {
// #nosec No risk for path injection. Reading static path of PCI data
file, err := os.Open(filepath.Join(basepath, pciAddress, "uevent"))
if err != nil {
return "", err
Expand Down
1 change: 1 addition & 0 deletions pkg/virt-handler/device-manager/mediated_device.go
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,7 @@ func (dpi *MediatedDevicePlugin) healthCheck() error {
}

func getMdevTypeName(mdevUUID string) (string, error) {
// #nosec No risk for path injection. Path is composed from static base "mdevBasePath" and static components
rawName, err := ioutil.ReadFile(filepath.Join(mdevBasePath, mdevUUID, "mdev_type/name"))
if err != nil {
return "", err
Expand Down
2 changes: 1 addition & 1 deletion pkg/virt-handler/selinux/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func (se *SELinuxImpl) selinux(args ...string) (out []byte, err error) {

func defaultCopyPolicyFunc(policyName string, dir string) (err error) {
sourceFile := "/" + policyName + ".cil"

// #nosec No risk for path injection. Using static string path
input, err := ioutil.ReadFile(sourceFile)
if err != nil {
return fmt.Errorf("failed to read a policy file %v: %v ", sourceFile, err)
Expand Down
4 changes: 2 additions & 2 deletions pkg/virt-handler/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ func (d *VirtualMachineController) getPodInterfacefromFileCache(uid types.UID, i
if exists {
return result, nil
}

// #nosec No risk for path injection. ifacepath is composed of static values from pkg/util
content, err := ioutil.ReadFile(ifacepath)
if err != nil {
log.Log.Reason(err).Errorf("failed to read from cache file: %s", err.Error())
Expand Down Expand Up @@ -2228,7 +2228,7 @@ func (d *VirtualMachineController) heartBeat(interval time.Duration, stopCh chan

func (d *VirtualMachineController) updateNodeCpuManagerLabel(cpuManagerPath string) {
var cpuManagerOptions map[string]interface{}

// #nosec No risk for path injection. cpuManagerPath is composed of static values from pkg/util
content, err := ioutil.ReadFile(cpuManagerPath)
if err != nil {
log.DefaultLogger().Reason(err).Errorf("failed to set a cpu manager label on host %s", d.host)
Expand Down
2 changes: 2 additions & 0 deletions pkg/virt-launcher/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ func (mon *monitor) RunForever(startTimeout time.Duration, signalStopChan chan s
mon.monitorLoop(startTimeout, signalStopChan)
}

// #nosec doesn't seem like this function is even used
func readProcCmdline(pathname string) ([]string, error) {
content, err := ioutil.ReadFile(pathname)
if err != nil {
Expand Down Expand Up @@ -221,6 +222,7 @@ func FindPid(commandNamePrefix string) (int, error) {
}

for _, entry := range entries {
// #nosec No risk for path injection. Reading specific entries under /proc
content, err := ioutil.ReadFile(entry)
if err != nil {
return 0, err
Expand Down
2 changes: 2 additions & 0 deletions pkg/virt-launcher/virtwrap/api/converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ func Convert_v1_Disk_To_api_Disk(diskDevice *v1.Disk, disk *Disk, devicePerBus m
func checkDirectIOFlag(path string) bool {
// check if fs where disk.img file is located or block device
// support direct i/o
// #nosec No risk for path injection. No information can be exposed to attacker
f, err := os.OpenFile(path, syscall.O_RDONLY|syscall.O_DIRECT, 0)
if err != nil && !os.IsNotExist(err) {
return false
Expand Down Expand Up @@ -1790,6 +1791,7 @@ func boolToString(value *bool, defaultPositive bool, positive string, negative s

// returns nameservers [][]byte, searchdomains []string, error
func GetResolvConfDetailsFromPod() ([][]byte, []string, error) {
// #nosec No risk for path injection. resolvConf is static "/etc/resolve.conf"
b, err := ioutil.ReadFile(resolvConf)
if err != nil {
return nil, nil, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/virt-launcher/virtwrap/util/libvirt_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ func StartVirtlog(stopChan chan struct{}, domainName string) {
}
time.Sleep(time.Second)
}

// #nosec No risk for path injection. logfile has a static basedir
file, err := os.Open(logfile)
if err != nil {
log.Log.Reason(err).Error("failed to catch virtlogd logs")
Expand Down
2 changes: 1 addition & 1 deletion pkg/virtctl/imageupload/imageupload.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ func (c *command) run(cmd *cobra.Command, args []string) error {
if err := parseArgs(args); err != nil {
return err
}

// #nosec No risk for path injection. This is used only to upload an image not to read info
file, err := os.Open(imagePath)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion pkg/watchdog/watchdog.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func WatchdogFileGetUID(baseDir string, vmi *v1.VirtualMachineInstance) string {
domain := precond.MustNotBeEmpty(vmi.GetObjectMeta().GetName())

filePath := WatchdogFileFromNamespaceName(baseDir, namespace, domain)

// #nosec No risk for path injection. Using static path and base path of "virtShareDir"
b, err := ioutil.ReadFile(filePath)
if err != nil {
return ""
Expand Down

0 comments on commit f68180d

Please sign in to comment.