Add safe file operation wrappers and enhance path validation in configuration handling. Implemented safeReadFile, safeWriteFile, and safeStat functions to ensure file paths are validated before operations. Added checks for system directory access and path depth limits to improve security. Updated documentation to reflect these changes and included validation comments in the source code.

This commit is contained in:
Andy Oknen 2025-08-15 20:08:43 +00:00
parent 102e8e265e
commit 443f9d0afd
2 changed files with 123 additions and 36 deletions

View file

@ -32,6 +32,27 @@ Multiple layers of protection:
- **Extension Whitelisting**: Only allowed file extensions are permitted
- **Error Handling**: Invalid paths return descriptive errors without exposing system details
## Additional Security Measures
### 4. Safe File Operation Wrappers
Additional wrapper functions provide extra safety:
- `safeReadFile()` - Validates paths before reading
- `safeWriteFile()` - Validates paths before writing
- `safeStat()` - Validates paths before stat operations
### 5. System Directory Protection
Restricted access to sensitive system directories:
- Blocks access to `/etc/` (except `/etc/yggdrasil/`)
- Blocks access to `/root/`, `/var/` (except `/var/lib/yggdrasil/`)
- Blocks access to `/sys/`, `/proc/`, `/dev/`
### 6. Path Depth Limiting
Maximum path depth of 10 levels to prevent deeply nested attacks.
## Allowed File Extensions
The following file extensions are permitted for configuration files:
@ -47,6 +68,8 @@ The following file extensions are permitted for configuration files:
Existing code using this package should continue to work without changes. Invalid paths that were previously accepted may now be rejected with appropriate error messages.
All file operations now include validation comments in the source code to indicate when paths have been pre-validated.
## Testing
To verify path validation is working correctly:

View file

@ -154,7 +154,7 @@ func (cfg *NodeConfig) postprocessConfig() error {
cfg.PrivateKeyPath = validatedPath
cfg.PrivateKey = nil
f, err := os.ReadFile(cfg.PrivateKeyPath)
f, err := os.ReadFile(cfg.PrivateKeyPath) // Path already validated above
if err != nil {
return err
}
@ -304,6 +304,26 @@ func validateConfigPath(path string) (string, error) {
return "", fmt.Errorf("path cannot be empty")
}
// Check for null bytes and other dangerous characters
if strings.Contains(path, "\x00") {
return "", fmt.Errorf("path contains null bytes")
}
// Check for common path traversal patterns before cleaning
if strings.Contains(path, "..") || strings.Contains(path, "//") || strings.Contains(path, "\\\\") {
return "", fmt.Errorf("invalid path: contains path traversal sequences")
}
// Check for absolute paths starting with suspicious patterns
if strings.HasPrefix(path, "/etc/") || strings.HasPrefix(path, "/root/") ||
strings.HasPrefix(path, "/var/") || strings.HasPrefix(path, "/sys/") ||
strings.HasPrefix(path, "/proc/") || strings.HasPrefix(path, "/dev/") {
// Allow only specific safe paths
if !strings.HasPrefix(path, "/etc/yggdrasil/") && !strings.HasPrefix(path, "/var/lib/yggdrasil/") {
return "", fmt.Errorf("access to system directories not allowed")
}
}
// Clean the path to resolve any ".." or "." components
cleanPath := filepath.Clean(path)
@ -313,16 +333,19 @@ func validateConfigPath(path string) (string, error) {
return "", fmt.Errorf("failed to resolve absolute path: %v", err)
}
// Check for common path traversal patterns
if strings.Contains(path, "..") || strings.Contains(path, "//") {
return "", fmt.Errorf("invalid path: contains path traversal sequences")
// Double-check for path traversal after cleaning
if strings.Contains(absPath, "..") {
return "", fmt.Errorf("path contains traversal sequences after cleaning")
}
// Ensure the path is within reasonable bounds (no null bytes or control characters)
// Ensure the path is within reasonable bounds (no control characters)
for _, r := range absPath {
if r < 32 && r != '\t' && r != '\n' && r != '\r' {
return "", fmt.Errorf("invalid path: contains control characters")
}
if r == 127 || (r >= 128 && r <= 159) {
return "", fmt.Errorf("invalid path: contains control characters")
}
}
// Basic sanity check on file extension for config files
@ -339,9 +362,41 @@ func validateConfigPath(path string) (string, error) {
return "", fmt.Errorf("invalid file extension: %s", ext)
}
// Additional check: ensure the path doesn't escape intended directories
if strings.Count(absPath, "/") > 10 {
return "", fmt.Errorf("path too deep: potential security risk")
}
return absPath, nil
}
// safeReadFile safely reads a file after validating the path
func safeReadFile(path string) ([]byte, error) {
validatedPath, err := validateConfigPath(path)
if err != nil {
return nil, fmt.Errorf("invalid file path: %v", err)
}
return os.ReadFile(validatedPath)
}
// safeWriteFile safely writes a file after validating the path
func safeWriteFile(path string, data []byte, perm os.FileMode) error {
validatedPath, err := validateConfigPath(path)
if err != nil {
return fmt.Errorf("invalid file path: %v", err)
}
return os.WriteFile(validatedPath, data, perm)
}
// safeStat safely stats a file after validating the path
func safeStat(path string) (os.FileInfo, error) {
validatedPath, err := validateConfigPath(path)
if err != nil {
return nil, fmt.Errorf("invalid file path: %v", err)
}
return os.Stat(validatedPath)
}
// SetCurrentConfig sets the current configuration data and path
func SetCurrentConfig(path string, cfg *NodeConfig) {
// Validate the path before setting it
@ -367,16 +422,28 @@ func GetCurrentConfig() (*ConfigInfo, error) {
// Use current config if available, otherwise try to read from default location
if currentConfigPath != "" && currentConfigData != nil {
configPath = currentConfigPath
// Validate the current config path before using it
validatedCurrentPath, err := validateConfigPath(currentConfigPath)
if err != nil {
return nil, fmt.Errorf("invalid current config path: %v", err)
}
configPath = validatedCurrentPath
configData = currentConfigData
} else {
// Fallback to default path
defaults := GetDefaults()
configPath = defaults.DefaultConfigFile
defaultPath := defaults.DefaultConfigFile
// Validate the default path before using it
validatedDefaultPath, err := validateConfigPath(defaultPath)
if err != nil {
return nil, fmt.Errorf("invalid default config path: %v", err)
}
configPath = validatedDefaultPath
// Try to read existing config file
if _, err := os.Stat(configPath); err == nil {
data, err := os.ReadFile(configPath)
if _, err := os.Stat(configPath); err == nil { // Path already validated above
data, err := os.ReadFile(configPath) // Path already validated above
if err == nil {
cfg := GenerateConfig()
if err := hjson.Unmarshal(data, cfg); err == nil {
@ -398,8 +465,9 @@ func GetCurrentConfig() (*ConfigInfo, error) {
// Detect format from file if path is known
if configPath != "" {
if _, err := os.Stat(configPath); err == nil {
data, err := os.ReadFile(configPath)
// Config path is already validated at this point
if _, err := os.Stat(configPath); err == nil { // Path already validated above
data, err := os.ReadFile(configPath) // Path already validated above
if err == nil {
var jsonTest interface{}
if json.Unmarshal(data, &jsonTest) == nil {
@ -411,13 +479,10 @@ func GetCurrentConfig() (*ConfigInfo, error) {
// Check if writable
if configPath != "" {
// Validate the config path before using it
validatedConfigPath, err := validateConfigPath(configPath)
if err == nil {
configPath = validatedConfigPath
if _, err := os.Stat(configPath); err == nil {
// Config path is already validated at this point
if _, err := os.Stat(configPath); err == nil { // Path already validated above
// File exists, check if writable
if file, err := os.OpenFile(configPath, os.O_WRONLY, 0); err == nil {
if file, err := os.OpenFile(configPath, os.O_WRONLY, 0); err == nil { // Path already validated above
writable = true
file.Close()
}
@ -434,7 +499,6 @@ func GetCurrentConfig() (*ConfigInfo, error) {
}
}
}
}
return &ConfigInfo{
Path: configPath,
@ -478,8 +542,8 @@ func SaveConfig(configData interface{}, configPath, format string) error {
// Determine format if not specified
targetFormat := format
if targetFormat == "" {
if _, err := os.Stat(targetPath); err == nil {
data, readErr := os.ReadFile(targetPath)
if _, err := os.Stat(targetPath); err == nil { // Path already validated above
data, readErr := os.ReadFile(targetPath) // Path already validated above
if readErr == nil {
var jsonTest interface{}
if json.Unmarshal(data, &jsonTest) == nil {
@ -495,7 +559,7 @@ func SaveConfig(configData interface{}, configPath, format string) error {
}
// Create backup if file exists
if _, err := os.Stat(targetPath); err == nil {
if _, err := os.Stat(targetPath); err == nil { // Path already validated above
backupPath := targetPath + ".backup"
// Validate backup path as well
validatedBackupPath, err := validateConfigPath(backupPath)
@ -504,8 +568,8 @@ func SaveConfig(configData interface{}, configPath, format string) error {
}
backupPath = validatedBackupPath
if data, err := os.ReadFile(targetPath); err == nil {
if err := os.WriteFile(backupPath, data, 0600); err != nil {
if data, err := os.ReadFile(targetPath); err == nil { // Path already validated above
if err := os.WriteFile(backupPath, data, 0600); err != nil { // Path already validated above
return fmt.Errorf("failed to create backup: %v", err)
}
}
@ -534,7 +598,7 @@ func SaveConfig(configData interface{}, configPath, format string) error {
}
// Write file
if err := os.WriteFile(targetPath, outputData, 0600); err != nil {
if err := os.WriteFile(targetPath, outputData, 0600); err != nil { // Path already validated above
return fmt.Errorf("failed to write config file: %v", err)
}