Skip to content

Commit

Permalink
gosec: extra: reorganize readdir validation
Browse files Browse the repository at this point in the history
Add validation for `os.ReadDir` much like
we did for `os.ReadFile`.

This was not required by gosec, but we
do for consistency and trying to be future-proof.

Signed-off-by: Francesco Romani <[email protected]>
(cherry picked from commit c712cce)
  • Loading branch information
ffromani committed Dec 2, 2024
1 parent 179fb76 commit b6e720b
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/config/cfgfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func fromDaemonFiles(pArgs *ProgArgs, configPathRoot string) error {

// this directory may be missing, that's expected and fine
configletDir := filepath.Join(configPathRoot, "daemon", "config.yaml.d")
if configlets, err := os.ReadDir(configletDir); err == nil {
if configlets, err := ReadConfigletDir(configletDir); err == nil {
for _, configlet := range configlets {
if !configlet.Type().IsRegular() {
klog.Infof("configlet %q not regular file: ignored", configlet.Name())
Expand Down
25 changes: 25 additions & 0 deletions pkg/config/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,31 @@ func Validate(pArgs *ProgArgs) error {
return nil
}

func ReadConfigletDir(configPath string) ([]fs.DirEntry, error) {
// to make gosec happy, the validation logic must be in the same function on which we call `os.ReadFile`.
// IOW, it seems the linter cannot track variable sanitization across functions.
configRoot := filepath.Clean(configPath)
cfgRoot, err := filepath.EvalSymlinks(configRoot)
if err != nil {
if errors.Is(err, fs.ErrNotExist) {
// reset to original value, it somehow passed the symlink check
cfgRoot = configRoot
} else {
return nil, fmt.Errorf("failed to validate config path: %w", err)
}
}
// else either success or checking a non-existing path. Which can be still OK.

pattern, err := IsConfigRootAllowed(cfgRoot, UserRunDir, UserHomeDir)
if err != nil {
return nil, err
}
if pattern == "" {
return nil, fmt.Errorf("failed to validate configRoot path %q: does not match any allowed pattern", cfgRoot)
}
return os.ReadDir(cfgRoot)
}

func ReadConfiglet(configPath string) ([]byte, error) {
// to make gosec happy, the validation logic must be in the same function on which we call `os.ReadFile`.
// IOW, it seems the linter cannot track variable sanitization across functions.
Expand Down

0 comments on commit b6e720b

Please sign in to comment.