Go: Why is DotDotCheck modeled as a complete path-injection barrier?

[wildoranges]

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

[oscarsj]

Thanks for reporting this — excellent question and analysis.

You’re right that !strings.Contains(path, "..") does not prevent all path-injection cases (for example absolute paths), so treating that check as a complete barrier may be too strong in some flows. We’ll review the DotDotCheck modeling and associated test expectations you referenced.

To help us reproduce and evaluate the right fix, could you share:

1. The exact query suite/pack you ran (for example go-security-extended vs custom).
2. A minimal standalone repro file and the exact codeql database analyze command.
3. Whether you observe the same suppression only on direct reads (like ReadFile(p)) or also when p is combined with a trusted base path.
4. Your expected behavior: should this be modeled as a partial guard only, or have no sanitizer effect unless combined with stronger constraints.

If useful, we can also propose a concrete model/test change in this issue once we confirm repro details.

[wildoranges]

@oscarsj Thanks. Here are the details.

1. I ran go/ql/src/Security/CWE-022/TaintedPath.ql directly, not a query suite.

• CodeQL repository: f6f45d1536312f53eed079868e344a5906bf3d72
• Source pack: codeql/go-queries@1.6.6-dev
• CodeQL CLI: 2.25.6
• Go: 1.22.12 linux/amd64

2. Minimal reproduction:

package main

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

func guarded(w http.ResponseWriter, r *http.Request) {
	p := r.URL.Query().Get("path")

	if !strings.Contains(p, "..") {
		data, err := os.ReadFile(p)
		if err != nil {
			http.Error(w, err.Error(), http.StatusInternalServerError)
			return
		}
		w.Write(data)
	}
}

func reference(w http.ResponseWriter, r *http.Request) {
	p := r.URL.Query().Get("path")
	data, _ := os.ReadFile(p)
	w.Write(data)
}

func main() {
	http.HandleFunc("/guarded", guarded)
	http.HandleFunc("/reference", reference)
	http.ListenAndServe(":8080", nil)
}

Database creation and analysis:

# Run from the directory containing main.go and go.mod
GOFLAGS=-mod=mod codeql database create /tmp/spike-db \
  --language=go \
  --source-root=. \
  --overwrite

# Run from the root of the github/codeql checkout
codeql database analyze /tmp/spike-db \
  go/ql/src/Security/CWE-022/TaintedPath.ql \
  --format=sarifv2.1.0 \
  --output=/tmp/spike.sarif \
  --rerun

The reference read is reported, while the guarded read is not.

The guarded branch can be exercised with an absolute path that contains no ..:

go run .
curl --path-as-is 'http://127.0.0.1:8080/guarded?path=/etc/passwd'

This passes the guard and reads /etc/passwd.

3. I confirmed the suppression on direct reads only. I have not confirmed the same behavior with p combined with a trusted base path.

4. I expect !strings.Contains(p, "..") alone to have no complete sanitizer effect. It should only contribute to a sanitizer when combined with stronger constraints, such as verifying that the path is relative/local or remains within a trusted root.