- Notifications
You must be signed in to change notification settings - Fork1.7k
Golang: Mark filepath.IsLocal as a tainted-path sanitizer guard#20056
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This pull request adds support for recognizingfilepath.IsLocal() as a path traversal sanitizer in the Go security analysis. The change treats calls to this function as a security guard that can prevent path traversal attacks when the function returns true.
- Adds a new
IsLocalChecksanitizer guard class to recognizefilepath.IsLocal()calls - Updates test cases to include an example of the sanitized path usage pattern
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll | Implements theIsLocalCheck class to treatfilepath.IsLocal() as a tainted-path sanitizer guard |
| go/ql/test/query-tests/Security/CWE-022/TaintedPath.go | Adds test case demonstrating proper usage offilepath.IsLocal() as a security check |
| if filepath.IsLocal(tainted_path) { | ||
| data, _ = ioutil.ReadFile(tainted_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The filepath.IsLocal() check alone may not provide complete path traversal protection. Consider combining it with additional validation such as filepath.Clean() or checking against an allowlist of permitted directories, as IsLocal() only validates that the path doesn't escape the current directory but doesn't prevent access to sensitive files within it.
| iffilepath.IsLocal(tainted_path) { | |
| data,_=ioutil.ReadFile(tainted_path) | |
| cleanedPath:=filepath.Clean(tainted_path) | |
| allowedDir:="/allowed/directory"// Replace with the actual allowed directory | |
| iffilepath.IsLocal(cleanedPath)&&strings.HasPrefix(cleanedPath,allowedDir) { | |
| data,_=ioutil.ReadFile(cleanedPath) |
Copilot uses AI. Check for mistakes.
16f3fc6 intomainUh oh!
There was an error while loading.Please reload this page.
No description provided.