-
Notifications
You must be signed in to change notification settings - Fork 35
Description
Summary
gosec flags G304 (CWE-22) because ReadFile() reads a file using a variable filename (os.ReadFile(filename)) without validation or restriction. If filename is influenced by untrusted input (CLI args, config, env vars, API input), this can enable path traversal / unintended file reads, leading to information disclosure (e.g., reading secrets or system files).
Location
onos-topo/pkg/tools/topo-generator/reader/reader.go:69(inReadFile(filename string))
Evidence (code)
Current implementation:
// ReadFile converts the human-readable file into the struct system above
func ReadFile(filename string) NetworkLayer {
// reading in the human-readable schema
var result NetworkLayer
content, err := os.ReadFile(filename)
if err != nil {
log.Error(err.Error())
return result
}
err = yaml.Unmarshal(content, &result)
if err != nil {
log.Error("Failed to parse file ", err)
}
return result
}Why this Matters:
os.ReadFile(filename) will read any path the process has access to. Without sanitization, a caller that can control filename could supply:
relative traversal paths like ../../../../etc/passwd
absolute paths like /var/run/secrets/...
symlink-based escapes to reach unintended files
This can result in information disclosure and data leakage, especially if the tool is used in shared environments, CI runners, containers, or if input comes from user-provided configs.
Suggested fix
Constrain reads to an allowed base directory and reject paths that escape it. One common pattern is:
filepath.Clean the user-provided path
resolve to an absolute path
ensure it stays under an allowed base directory
Example:
func ReadFile(filename string) (NetworkLayer, error) {
var result NetworkLayer
baseDir := "/var/topo-generator" // example allowed directory
clean := filepath.Clean(filename)
absBase, err := filepath.Abs(baseDir)
if err != nil {
return result, err
}
absPath, err := filepath.Abs(filepath.Join(absBase, clean))
if err != nil {
return result, err
}
if !strings.HasPrefix(absPath, absBase+string(os.PathSeparator)) {
return result, fmt.Errorf("invalid path: escapes base directory")
}
content, err := os.ReadFile(absPath)
if err != nil {
return result, err
}
if err := yaml.Unmarshal(content, &result); err != nil {
return result, err
}
return result, nil
}