Address security concerns reported by 'gosec'

gosec reports several issues, none of them looks very critical. With
this change the following concerns have been addressed:

[pkg/cephfs/nodeserver.go:229] - G302: Expect file permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)
  > os.Chmod(targetPath, 0777)

[pkg/cephfs/util.go:39] - G204: Subprocess launched with variable (Confidence: HIGH, Severity: MEDIUM)
  > exec.Command(program, args...)

[pkg/rbd/nodeserver.go:156] - G302: Expect file permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)
  > os.Chmod(stagingTargetPath, 0777)

[pkg/rbd/nodeserver.go:205] - G302: Expect file permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)
  > os.OpenFile(mountPath, os.O_CREATE|os.O_RDWR, 0750)

[pkg/rbd/rbd_util.go:797] - G304: Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
  > ioutil.ReadFile(fPath)

[pkg/util/cephcmds.go:35] - G204: Subprocess launched with variable (Confidence: HIGH, Severity: MEDIUM)
  > exec.Command(program, args...)

[pkg/util/credentials.go:47] - G104: Errors unhandled. (Confidence: HIGH, Severity: LOW)
  > os.Remove(tmpfile.Name())

[pkg/util/credentials.go:92] - G104: Errors unhandled. (Confidence: HIGH, Severity: LOW)
  > os.Remove(cr.KeyFile)

[pkg/util/pidlimit.go:74] - G304: Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
  > os.Open(pidsMax)

URL: https://github.com/securego/gosec
Signed-off-by: Niels de Vos <ndevos@redhat.com>
This commit is contained in:
Niels de Vos 2019-08-30 12:23:10 +02:00 committed by mergify[bot]
parent d6f1c938d8
commit dd668e59f1
7 changed files with 11 additions and 7 deletions

View File

@ -226,6 +226,7 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
klog.Infof(util.Log(ctx, "cephfs: successfully bind-mounted volume %s to %s"), volID, targetPath) klog.Infof(util.Log(ctx, "cephfs: successfully bind-mounted volume %s to %s"), volID, targetPath)
// #nosec - allow anyone to write inside the target path
err = os.Chmod(targetPath, 0777) err = os.Chmod(targetPath, 0777)
if err != nil { if err != nil {
klog.Errorf(util.Log(ctx, "failed to change targetpath permission for volume %s: %v"), volID, err) klog.Errorf(util.Log(ctx, "failed to change targetpath permission for volume %s: %v"), volID, err)

View File

@ -36,7 +36,7 @@ type volumeID string
func execCommand(ctx context.Context, program string, args ...string) (stdout, stderr []byte, err error) { func execCommand(ctx context.Context, program string, args ...string) (stdout, stderr []byte, err error) {
var ( var (
cmd = exec.Command(program, args...) // nolint: gosec cmd = exec.Command(program, args...) // nolint: gosec, #nosec
sanitizedArgs = util.StripSecretInArgs(args) sanitizedArgs = util.StripSecretInArgs(args)
stdoutBuf bytes.Buffer stdoutBuf bytes.Buffer
stderrBuf bytes.Buffer stderrBuf bytes.Buffer

View File

@ -153,6 +153,7 @@ func (ns *NodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
} }
isMounted = true isMounted = true
// #nosec - allow anyone to write inside the target path
err = os.Chmod(stagingTargetPath, 0777) err = os.Chmod(stagingTargetPath, 0777)
if err != nil { if err != nil {
return nil, status.Error(codes.Internal, err.Error()) return nil, status.Error(codes.Internal, err.Error())
@ -202,7 +203,7 @@ func (ns *NodeServer) undoStagingTransaction(ctx context.Context, stagingParentP
func (ns *NodeServer) createStageMountPoint(ctx context.Context, mountPath string, isBlock bool) error { func (ns *NodeServer) createStageMountPoint(ctx context.Context, mountPath string, isBlock bool) error {
if isBlock { if isBlock {
pathFile, err := os.OpenFile(mountPath, os.O_CREATE|os.O_RDWR, 0750) pathFile, err := os.OpenFile(mountPath, os.O_CREATE|os.O_RDWR, 0600)
if err != nil { if err != nil {
klog.Errorf(util.Log(ctx, "failed to create mountPath:%s with error: %v"), mountPath, err) klog.Errorf(util.Log(ctx, "failed to create mountPath:%s with error: %v"), mountPath, err)
return status.Error(codes.Internal, err.Error()) return status.Error(codes.Internal, err.Error())

View File

@ -794,7 +794,7 @@ func lookupRBDImageMetadataStash(path string) (rbdImageMetadataStash, error) {
var imgMeta rbdImageMetadataStash var imgMeta rbdImageMetadataStash
fPath := filepath.Join(path, stashFileName) fPath := filepath.Join(path, stashFileName)
encodedBytes, err := ioutil.ReadFile(fPath) encodedBytes, err := ioutil.ReadFile(fPath) // #nosec - intended reading from fPath
if err != nil { if err != nil {
if !os.IsNotExist(err) { if !os.IsNotExist(err) {
return imgMeta, fmt.Errorf("failed to read stashed JSON image metadata from path (%s): (%v)", fPath, err) return imgMeta, fmt.Errorf("failed to read stashed JSON image metadata from path (%s): (%v)", fPath, err)

View File

@ -32,7 +32,7 @@ import (
// ExecCommand executes passed in program with args and returns separate stdout and stderr streams // ExecCommand executes passed in program with args and returns separate stdout and stderr streams
func ExecCommand(program string, args ...string) (stdout, stderr []byte, err error) { func ExecCommand(program string, args ...string) (stdout, stderr []byte, err error) {
var ( var (
cmd = exec.Command(program, args...) // nolint: gosec cmd = exec.Command(program, args...) // nolint: gosec, #nosec
sanitizedArgs = StripSecretInArgs(args) sanitizedArgs = StripSecretInArgs(args)
stdoutBuf bytes.Buffer stdoutBuf bytes.Buffer
stderrBuf bytes.Buffer stderrBuf bytes.Buffer

View File

@ -44,7 +44,8 @@ func storeKey(key string) (string, error) {
} }
defer func() { defer func() {
if err != nil { if err != nil {
os.Remove(tmpfile.Name()) // don't complain about unhandled error
_ = os.Remove(tmpfile.Name())
} }
}() }()
@ -89,7 +90,8 @@ func newCredentialsFromSecret(idField, keyField string, secrets map[string]strin
} }
func (cr *Credentials) DeleteCredentials() { func (cr *Credentials) DeleteCredentials() {
os.Remove(cr.KeyFile) // don't complain about unhandled error
_ = os.Remove(cr.KeyFile)
} }
func NewUserCredentials(secrets map[string]string) (*Credentials, error) { func NewUserCredentials(secrets map[string]string) (*Credentials, error) {

View File

@ -71,7 +71,7 @@ func GetPIDLimit() (int, error) {
return 0, err return 0, err
} }
f, err := os.Open(pidsMax) f, err := os.Open(pidsMax) // #nosec - intended reading from /sys/...
if err != nil { if err != nil {
return 0, err return 0, err
} }