Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 15, 2025

Conversation

smowton
Copy link
Contributor

@smowton smowton commented Jul 15, 2025

No description provided.

@Copilot Copilot AI review requested due to automatic review settings July 15, 2025 13:47
@smowton smowton requested a review from a team as a code owner July 15, 2025 13:47
@github-actions github-actions bot added the Go label Jul 15, 2025
Copy link
Contributor

@Copilot Copilot AI left a 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 recognizing filepath.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 IsLocalCheck sanitizer guard class to recognize filepath.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 the IsLocalCheck class to treat filepath.IsLocal() as a tainted-path sanitizer guard
go/ql/test/query-tests/Security/CWE-022/TaintedPath.go Adds test case demonstrating proper usage of filepath.IsLocal() as a security check

Comment on lines +98 to +99
if filepath.IsLocal(tainted_path) {
data, _ = ioutil.ReadFile(tainted_path)
Copy link
Preview

Copilot AI Jul 15, 2025

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.

Suggested change
if filepath.IsLocal(tainted_path) {
data, _ = ioutil.ReadFile(tainted_path)
cleanedPath := filepath.Clean(tainted_path)
allowedDir := "/allowed/directory" // Replace with the actual allowed directory
if filepath.IsLocal(cleanedPath) && strings.HasPrefix(cleanedPath, allowedDir) {
data, _ = ioutil.ReadFile(cleanedPath)

Copilot uses AI. Check for mistakes.

owen-mc
owen-mc previously approved these changes Jul 15, 2025
@smowton smowton merged commit 16f3fc6 into main Jul 15, 2025
17 checks passed
@smowton smowton deleted the smowton/fix/tainted-path-is-local branch July 15, 2025 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants