-
Notifications
You must be signed in to change notification settings - Fork 1
fix(security): validate sidecar paths to prevent path injection attacks #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -297,9 +297,9 @@ | |
| // compressible and large enough to benefit from compression. | ||
| if h.cfg.Compression.Enabled && h.cfg.Compression.Precompressed && | ||
| compress.IsCompressible(ct) && len(data) >= h.cfg.Compression.MinSize { | ||
| cached.GzipData = loadSidecar(absPath + ".gz") | ||
| cached.BrData = loadSidecar(absPath + ".br") | ||
| cached.ZstdData = loadSidecar(absPath + ".zst") | ||
| cached.GzipData = h.loadSidecar(absPath + ".gz") | ||
| cached.BrData = h.loadSidecar(absPath + ".br") | ||
| cached.ZstdData = h.loadSidecar(absPath + ".zst") | ||
| } | ||
|
|
||
| // Generate on-the-fly gzip if no sidecar and content is compressible. | ||
|
|
@@ -502,10 +502,46 @@ | |
| } | ||
|
|
||
| // loadSidecar attempts to read a pre-compressed sidecar file. | ||
| // Returns nil if the sidecar does not exist or cannot be read. | ||
| func loadSidecar(path string) []byte { | ||
| data, err := os.ReadFile(path) | ||
| // Returns nil if the sidecar does not exist, cannot be read, or fails validation. | ||
| // The path parameter must be constructed from a validated absolute filesystem path | ||
| // (e.g., absPath + ".gz") to ensure it remains within the root directory. | ||
| func (h *FileHandler) loadSidecar(path string) []byte { | ||
| // Resolve symlinks to get the canonical path. | ||
| // This prevents symlink escape attacks where a sidecar could point outside root. | ||
| realPath, err := filepath.EvalSymlinks(path) | ||
| if err != nil { | ||
| // File doesn't exist or can't be resolved — return nil. | ||
| if os.IsNotExist(err) { | ||
| return nil | ||
| } | ||
| // Other errors (permission denied, etc.) — treat as inaccessible. | ||
| return nil | ||
| } | ||
|
|
||
| // Resolve the root directory to its canonical path for comparison. | ||
| // This is important on platforms like macOS where /tmp → /private/tmp. | ||
| realRoot := h.absRoot | ||
| if r, err := filepath.EvalSymlinks(h.absRoot); err == nil { | ||
| realRoot = r | ||
| } | ||
|
|
||
| // Ensure the resolved sidecar path is still within the root directory. | ||
| // Add a trailing separator to prevent prefix collisions like "/root" matching "/rootsuffix". | ||
| rootWithSep := realRoot | ||
| if !strings.HasSuffix(rootWithSep, string(filepath.Separator)) { | ||
| rootWithSep += string(filepath.Separator) | ||
| } | ||
|
|
||
| // Reject if the sidecar path escapes the root directory. | ||
| if realPath != realRoot && !strings.HasPrefix(realPath, rootWithSep) { | ||
| // Sidecar path escapes the root — reject it. | ||
| return nil | ||
| } | ||
|
|
||
| // Path is validated and safe — read the file. | ||
| data, err := os.ReadFile(realPath) | ||
Check failureCode scanning / CodeQL Uncontrolled data used in path expression High
This path depends on a
user-provided value Error loading related location Loading Copilot AutofixAI 3 days ago Copilot could not generate an autofix suggestion Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support. |
||
| if err != nil { | ||
| // File doesn't exist or can't be read — return nil. | ||
| return nil | ||
| } | ||
| return data | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.