Go: Why is DotDotCheck modeled as a complete path-injection barrier?
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 50/100
Research direction
Start with DotDotCheck in TaintedPathCustomizations.qll (lines 106-122), then inspect the GOOD case at TaintedPath.go line 31 and the Go path-injection query tests. Verify how the false branch becomes an isBarrier through SanitizerGuardAsSanitizer, and compare the intended behavior with the filepath.Base guidance. Done means the model and affected test expectations consistently reflect the supported protection against path traversal.
Written by the indexing model from the issue text.
Description
Description of the issue
The Go go/path-injection query treats !strings.Contains(path, "..") as a complete path-injection sanitizer. This means that when Contains returns false, taint is fully blocked - no additional sanitizer is required.
I'm wondering why this is modeled as a complete barrier, because checking for the absence of ".." does not prevent absolute-path attacks. For example, /etc/passwd does not contain "..", passes the guard, and can read files outside any intended directory.
The query help seems to agree - it says this approach is "only suitable if the input is expected to be a single file name", and also warns that user-controlled paths "may be absolute paths". But DotDotCheck doesn't distinguish between single-component inputs and multi-component paths.
The existing test case at TaintedPath.go line 31 marks this pattern as GOOD with the comment "This can only read inside the provided safe path", but tainted_path = "/etc/passwd" would still pass the check and escape any safe path.
Affected sanitizer
DotDotCheck in TaintedPathCustomizations.qll (lines 106-122) matches strings.Contains(p, "..") and declares the false branch as a complete sanitizer guard. Through SanitizerGuardAsSanitizer, this becomes a full isBarrier node.
Steps to reproduce
func handler(w http.ResponseWriter, r *http.Request) {
p := r.URL.Query().Get("file")
if !strings.Contains(p, "..") {
data, _ := ioutil.ReadFile(p) // 0 alerts with DotDotCheck enabled
w.Write(data)
}
}
Question
Given that !strings.Contains(p, "..") only blocks one specific attack vector and not absolute paths, would it be more appropriate to model it as a non-barrier (or at least not a complete one), similar to how filepath.Base is documented as "not a sanitizer for path traversal" in mime.multipart.model.yml?
Environment
- CodeQL CLI 2.25.6
- CodeQL repository commit:
f6f45d1536312f53eed079868e344a5906bf3d72 - Go 1.22.12 on Linux/amd64
- Dominant language
- CodeQL
- Stars
- 10.1k
- Forks
- 2.1k
- Avg merge
- 2d 16h
- Merged PRs (30d)
- 143
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from github/codeql
-
agentic-workflows
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
-
false-positive javascript
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
-
C#: cs/simplifiable-boolean-expression false positive on Nullable<bool> compared with a literal Open
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
-
false-positive
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
Similar issues
-
enhancement
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
canonical/paas-charm#368 · 1 comment ·
-
enhancement
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
palladius/rails8-app-on-gcp#142 ·
-
Difficulty 1/5 Under an hour Newbie friendliness 90/100
StevenBlack/hosts#3256 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
corsairdev/corsair#1764 ·
-
oblt-aw/detector/security
Difficulty 2/5 1-3 hours Newbie friendliness 70/100