From c712cceeffb09e05c4ed7493ac4dd29eba072041 Mon Sep 17 00:00:00 2001
From: Francesco Romani <fromani@redhat.com>
Date: Mon, 2 Dec 2024 10:53:00 +0100
Subject: [PATCH] gosec: extra: reorganize readdir validation

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 <fromani@redhat.com>
---
 pkg/config/cfgfile.go    |  2 +-
 pkg/config/validation.go | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/pkg/config/cfgfile.go b/pkg/config/cfgfile.go
index 9a0bdbdf..f4c66b85 100644
--- a/pkg/config/cfgfile.go
+++ b/pkg/config/cfgfile.go
@@ -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())
diff --git a/pkg/config/validation.go b/pkg/config/validation.go
index 9fc36986..8fd34f33 100644
--- a/pkg/config/validation.go
+++ b/pkg/config/validation.go
@@ -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.