fix: add path validation to prevent directory traversal attacks (G304)

loonghao · loonghao · commit 55cd7f75fcdb · 2025-05-31T21:53:16.000+08:00
- Add validateConfigPath() function to validate config file paths
- Add validateFilePath() function to validate file copy operations
- Prevent directory traversal attacks with .. path components
- Restrict file operations to safe directories (working dir, user config, temp)
- Only allow .yaml/.yml extensions for config files
- Maintain test compatibility with temp directory access

Security improvements:
- G304 file inclusion vulnerabilities now have proper path validation
- Directory traversal attacks are prevented
- File operations are restricted to safe locations
- Config files are validated for safe extensions

Signed-off-by: longhao &lt;hal.long@outlook.com&gt;
diff --git a/internal/cli/start.go b/internal/cli/start.go
@@ -8,6 +8,7 @@ import (
 	"os/signal"
 	"path/filepath"
 	"runtime"
+	"strings"
 	"syscall"
 	"time"
 
@@ -330,6 +331,14 @@ func startGoServer(ctx context.Context, serverPort, executorPort string, verbose
 }
 
 func copyFile(src, dst string) error {
+	// Validate source and destination paths to prevent directory traversal
+	if err := validateFilePath(src, "source"); err != nil {
+		return fmt.Errorf("invalid source path: %w", err)
+	}
+	if err := validateFilePath(dst, "destination"); err != nil {
+		return fmt.Errorf("invalid destination path: %w", err)
+	}
+
 	sourceFile, err := os.ReadFile(src)
 	if err != nil {
 		return err
@@ -338,6 +347,37 @@ func copyFile(src, dst string) error {
 	return os.WriteFile(dst, sourceFile, 0600)
 }
 
+// validateFilePath validates that a file path is safe for operations
+func validateFilePath(path, pathType string) error {
+	// Clean the path to resolve any .. or . components
+	cleanPath := filepath.Clean(path)
+
+	// Check for directory traversal attempts
+	if strings.Contains(cleanPath, "..") {
+		return fmt.Errorf("directory traversal not allowed in %s path", pathType)
+	}
+
+	// Get absolute path for further validation
+	absPath, err := filepath.Abs(cleanPath)
+	if err != nil {
+		return fmt.Errorf("failed to get absolute path for %s: %w", pathType, err)
+	}
+
+	// Get current working directory
+	wd, err := os.Getwd()
+	if err != nil {
+		return fmt.Errorf("failed to get working directory: %w", err)
+	}
+
+	// Only allow files within current working directory or its subdirectories
+	wdAbs, _ := filepath.Abs(wd)
+	if !strings.HasPrefix(absPath, wdAbs) {
+		return fmt.Errorf("%s file must be within current working directory", pathType)
+	}
+
+	return nil
+}
+
 // parsePort parses a port string to integer
 func parsePort(portStr string) (int, error) {
 	if portStr == "" {
diff --git a/internal/config/config.go b/internal/config/config.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"gopkg.in/yaml.v3"
 
@@ -167,6 +168,11 @@ func (c *Config) setDefaults() {
 
 // loadFromFile loads configuration from YAML file
 func (c *Config) loadFromFile(path string) error {
+	// Validate file path to prevent directory traversal attacks
+	if err := validateConfigPath(path); err != nil {
+		return fmt.Errorf("invalid config path: %w", err)
+	}
+
 	data, err := os.ReadFile(path)
 	if err != nil {
 		return err
@@ -175,6 +181,59 @@ func (c *Config) loadFromFile(path string) error {
 	return yaml.Unmarshal(data, c)
 }
 
+// validateConfigPath validates that the config file path is safe
+func validateConfigPath(path string) error {
+	// Clean the path to resolve any .. or . components
+	cleanPath := filepath.Clean(path)
+
+	// Check for directory traversal attempts
+	if strings.Contains(cleanPath, "..") {
+		return fmt.Errorf("directory traversal not allowed")
+	}
+
+	// Only allow specific file extensions
+	ext := strings.ToLower(filepath.Ext(cleanPath))
+	if ext != ".yaml" && ext != ".yml" {
+		return fmt.Errorf("only .yaml and .yml files are allowed")
+	}
+
+	// Get absolute path for further validation
+	absPath, err := filepath.Abs(cleanPath)
+	if err != nil {
+		return fmt.Errorf("failed to get absolute path: %w", err)
+	}
+
+	// Get current working directory
+	wd, err := os.Getwd()
+	if err != nil {
+		return fmt.Errorf("failed to get working directory: %w", err)
+	}
+
+	// Allow files in current directory or user config directory
+	wdAbs, _ := filepath.Abs(wd)
+	if strings.HasPrefix(absPath, wdAbs) {
+		return nil
+	}
+
+	// Allow files in user config directory
+	if configDir, err := os.UserConfigDir(); err == nil {
+		configDirAbs, _ := filepath.Abs(configDir)
+		if strings.HasPrefix(absPath, configDirAbs) {
+			return nil
+		}
+	}
+
+	// Allow files in system temp directory (for testing)
+	if tempDir := os.TempDir(); tempDir != "" {
+		tempDirAbs, _ := filepath.Abs(tempDir)
+		if strings.HasPrefix(absPath, tempDirAbs) {
+			return nil
+		}
+	}
+
+	return fmt.Errorf("config file must be in current directory, user config directory, or temp directory")
+}
+
 // loadFromEnv loads configuration from environment variables
 func (c *Config) loadFromEnv() {
 	if host := os.Getenv("WEBHOOK_BRIDGE_HOST"); host != "" {
