Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions go/ql/src/Security/CWE-022/TaintedPath.qhelp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,20 @@ result in the string "../" if only "../" sequences are removed.
Finally, the simplest (but most restrictive) option is to use an allow list of safe patterns
and make sure that the user input matches one of these patterns.
</p>

<p>
<b>Sanitizer functions from the Go standard library:</b>
</p>
<p>
The Go standard library provides several functions that can help sanitize paths:
</p>
<ul>
<li><code>filepath.Rel(basepath, targpath)</code> - Returns a relative path from basepath to targpath. If the path is not contained within basepath, it returns an error, making it useful for validating that a path stays within expected boundaries.</li>
Copy link
Preview

Copilot AI Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This description is incorrect. filepath.Rel() does not return an error when the target path is outside the base path. It returns a relative path that may contain .. sequences to reach the target. The error checking should focus on whether the result contains .. prefixes, not just if an error is returned.

Suggested change
<li><code>filepath.Rel(basepath, targpath)</code> - Returns a relative path from basepath to targpath. If the path is not contained within basepath, it returns an error, making it useful for validating that a path stays within expected boundaries.</li>
<li><code>filepath.Rel(basepath, targpath)</code> - Returns a relative path from basepath to targpath. If the target path is outside the base path, the result may contain <code>..</code> components; it does <b>not</b> return an error in this case. To validate that a path stays within expected boundaries, check that the result does not start with <code>..</code> or contain <code>..</code> components.</li>

Copilot uses AI. Check for mistakes.

<li><code>filepath.Clean(path)</code> - When used with a path that starts with "/" (e.g., <code>filepath.Clean("/" + userPath)</code>), it normalizes the path and can help prevent path traversal by resolving ".." sequences.</li>
<li><code>filepath.IsLocal(path)</code> - Returns true if the path is local (doesn't start with "/" and doesn't contain ".."), making it useful for validating relative paths.</li>
<li><code>strings.Contains(path, "..")</code> - Can be used to detect path traversal sequences in user input.</li>
<li><code>strings.HasPrefix(path, safePrefix)</code> - Can be used to ensure paths start with expected safe prefixes.</li>
</ul>
</recommendation>

<example>
Expand Down Expand Up @@ -71,6 +85,13 @@ that the resulting path is still within it.
</p>
<sample src="TaintedPathGood2.go" />
<p>
<b>Using Go standard library sanitizers:</b>
</p>
<p>
You can also use Go's built-in path validation functions for additional safety:
</p>
<sample src="TaintedPathSanitizers.go" />
<p>
Note that <code>/home/user</code> is just an example, you should replace it with the actual
safe directory in your application. Also, while in this example the path of the safe
directory is absolute, this may not always be the case, and you may need to resolve it
Expand Down
56 changes: 56 additions & 0 deletions go/ql/src/Security/CWE-022/TaintedPathSanitizers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package main

import (
"net/http"
"os"
"path/filepath"
"strings"
)

func handleFileWithSanitizers(w http.ResponseWriter, r *http.Request) {
userPath := r.URL.Query().Get("file")
safeDir := "/home/user/documents"

// Method 1: Using filepath.IsLocal to validate the path is local
if !filepath.IsLocal(userPath) {
http.Error(w, "Invalid path: must be local", http.StatusBadRequest)
return
}

// Method 2: Using strings.Contains to check for path traversal sequences
if strings.Contains(userPath, "..") {
http.Error(w, "Invalid path: contains path traversal", http.StatusBadRequest)
return
}

// Method 3: Using filepath.Rel to ensure path is within safe directory
relPath, err := filepath.Rel(safeDir, filepath.Join(safeDir, userPath))
if err != nil || strings.HasPrefix(relPath, "..") {
http.Error(w, "Invalid path: outside safe directory", http.StatusBadRequest)
return
}

// Method 4: Using filepath.Clean with absolute prefix for normalization
cleanPath := filepath.Clean("/" + userPath)
if strings.Contains(cleanPath, "..") {
http.Error(w, "Invalid path after normalization", http.StatusBadRequest)
return
}

// Method 5: Using strings.HasPrefix for additional validation
finalPath := filepath.Join(safeDir, userPath)
if !strings.HasPrefix(finalPath, safeDir) {
Copy link
Preview

Copilot AI Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This validation method is vulnerable to bypass. An attacker could use a path like ../documents which, when joined with /home/user/documents, becomes /home/user/documents/../documents and still has the safe directory as a prefix even though it references a parent directory. This check should occur after filepath.Clean() or use a more robust validation method.

Suggested change
if !strings.HasPrefix(finalPath, safeDir) {
absFinalPath, err := filepath.Abs(finalPath)
if err != nil {
http.Error(w, "Invalid path: could not resolve absolute path", http.StatusBadRequest)
return
}
absSafeDir, err := filepath.Abs(safeDir)
if err != nil {
http.Error(w, "Invalid safe directory", http.StatusInternalServerError)
return
}
if !strings.HasPrefix(absFinalPath, absSafeDir+string(os.PathSeparator)) && absFinalPath != absSafeDir {

Copilot uses AI. Check for mistakes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect: the result of filepath.Join is Clean'd.

http.Error(w, "Invalid path: must be within safe directory", http.StatusBadRequest)
return
}

// Safe to open file
file, err := os.Open(finalPath)
Comment on lines +26 to +48
Copy link
Preview

Copilot AI Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sanitization method has a logical flaw. The filepath.Join(safeDir, userPath) operation happens before validation, which means a malicious userPath like ../../../etc/passwd could escape the safe directory before filepath.Rel is called. Consider validating userPath first or using filepath.Rel(safeDir, userPath) directly to check if the user path would escape the safe directory.

Suggested change
// Method 3: Using filepath.Rel to ensure path is within safe directory
relPath, err := filepath.Rel(safeDir, filepath.Join(safeDir, userPath))
if err != nil || strings.HasPrefix(relPath, "..") {
http.Error(w, "Invalid path: outside safe directory", http.StatusBadRequest)
return
}
// Method 4: Using filepath.Clean with absolute prefix for normalization
cleanPath := filepath.Clean("/" + userPath)
if strings.Contains(cleanPath, "..") {
http.Error(w, "Invalid path after normalization", http.StatusBadRequest)
return
}
// Method 5: Using strings.HasPrefix for additional validation
finalPath := filepath.Join(safeDir, userPath)
if !strings.HasPrefix(finalPath, safeDir) {
http.Error(w, "Invalid path: must be within safe directory", http.StatusBadRequest)
return
}
// Safe to open file
file, err := os.Open(finalPath)
// Robust: Resolve absolute path and ensure it's within safeDir
absSafeDir, err := filepath.Abs(safeDir)
if err != nil {
http.Error(w, "Internal error: cannot resolve safe directory", http.StatusInternalServerError)
return
}
absFilePath, err := filepath.Abs(filepath.Join(absSafeDir, userPath))
if err != nil {
http.Error(w, "Invalid path", http.StatusBadRequest)
return
}
rel, err := filepath.Rel(absSafeDir, absFilePath)
if err != nil || strings.HasPrefix(rel, "..") || filepath.IsAbs(rel) {
http.Error(w, "Invalid path: outside safe directory", http.StatusBadRequest)
return
}
// Safe to open file
file, err := os.Open(absFilePath)

Copilot uses AI. Check for mistakes.

if err != nil {
http.Error(w, "File not found", http.StatusNotFound)
return
}
defer file.Close()

// Process file...
}