path/filepath.Rel (Go) modeled as path-injection barrier causes false-negative path-traversal alerts

[wildoranges]

Description of the issue

path/filepath.Rel (Go) is modeled as a barrierModel for path-injection in go/ql/lib/ext/path.filepath.model.yml, but filepath.Rel is not a path-injection sanitizer — it returns a relative path that may contain .. components, which can traverse outside the intended base directory. This causes CodeQL to miss path-traversal alerts (false negatives) on code that uses filepath.Rel to "sanitize" user-controlled paths.

CodeQL's own test suite marks two such cases as GOOD (non-vulnerable), but they appear to be real path-traversal vulnerabilities. I confirmed the false negatives by removing the barrierModel row and re-running the tests — the missing alerts appear.

The model

go/ql/lib/ext/path.filepath.model.yml (the barrierModel section, lines 1-6):

extensions:
  - addsTo:
      pack: codeql/go-all
      extensible: barrierModel
    data:
      - ["path/filepath", "", False, "Rel", "", "", "ReturnValue", "path-injection", "manual"]

This means CodeQL considers the return value of filepath.Rel to be safe from path-injection (taint of kind path-injection is stopped at the barrier).

Why filepath.Rel is not a path-injection sanitizer

Per the Go docs, filepath.Rel(basePath, targPath) "returns a relative path that is lexically equivalent to targPath when joined to basePath", and "the returned path will always be relative to basePath, even if basePath and targPath share no elements." The docs' own example output shows the result can contain ..:

filepath.Rel("/a", "/b/c")  // "../b/c", nil   (this is the official doc example output)

Rel is purely lexical — it does not resolve symlinks and does not confine the result under basepath. When targpath is outside basepath, the result necessarily begins with ... Verified by running Go 1.22.12:

filepath.Rel("/home/user/safepath", "/home/user/etc/passwd")  // "../etc/passwd", nil
filepath.Rel("/home/user/safepath", "/etc/passwd")            // "../../../etc/passwd", nil
filepath.Join("/home/user/safepath", "../../../etc/passwd")   // "/etc/passwd"  — escapes the base directory

The result can traverse outside the base directory, so the return value is not safe from path-injection and should not be a barrierModel.

Notably, filepath.Base is explicitly documented in CodeQL's own model files as not being a path-traversal sanitizer (go/ql/lib/ext/mime.multipart.model.yml):

path/filepath.Base is not a sanitizer for path traversal, but in this specific case where the output is going to be used as a filename rather than a directory name, it is adequate.

Rel is no stronger than Base as a sanitizer (both return paths that may traverse), yet Rel is marked as a path-injection barrier while Base is explicitly noted as not a sanitizer.

Confirmed false negatives (with ablation)

Case 1: TaintedPath.go line 27

go/ql/test/query-tests/Security/CWE-022/TaintedPath.go lines 24-28:

// GOOD: This can only read inside the provided safe path
sanitized_filepath, _ := filepath.Rel("/home/user/safepath", tainted_path)
data, _ = ioutil.ReadFile(sanitized_filepath)
w.Write(data)

• tainted_path is a confirmed source (r.URL.Query()["path"][0], line 15, annotated // $ Source[go/path-injection])
• ioutil.ReadFile is a confirmed sink (Sink: io/ioutil; ; false; ReadFile; ; ; Argument[0]; path-injection; manual in TaintedPath.expected)
• TaintedPath.expected reports path-injection alerts for lines 18, 22, and 74, but not for line 27 (ReadFile(sanitized_filepath))

Ablation confirmation: I removed the filepath.Rel barrierModel row and re-ran the test. The .actual output now includes line 27:

| TaintedPath.go:27:28:27:45 | sanitized_filepath | TaintedPath.go:15:18:15:22 | selection of URL | TaintedPath.go:27:28:27:45 | sanitized_filepath | This path depends on a $@. | TaintedPath.go:15:18:15:22 | selection of URL | user-provided value |

This confirms the barrierModel is suppressing a legitimate path-injection alert. The taint flows tainted_path → filepath.Rel → sanitized_filepath → ioutil.ReadFile, and sanitized_filepath can be "../etc/passwd" (a traversal path).

Case 2: tst.go line 27 (ZipSlip test)

go/ql/test/query-tests/Security/CWE-022/tst.go lines 23-28:

for _, f := range r.File {
    path := f.Name
    relpath, err := filepath.Rel(root, path)
    if err == nil {
        ioutil.WriteFile(filepath.Join(root, relpath), []byte("present"), 0666) // OK
    }

This is marked // OK, but relpath = filepath.Rel(root, path) can contain .. when path is an absolute target outside root (e.g. if path = "/etc/passwd", then filepath.Rel("/tmp/extract", "/etc/passwd") returns "../../etc/passwd", and filepath.Join(root, relpath) resolves to /etc/passwd, traversing outside root). Verified by running Go 1.22.12:

filepath.Rel("/tmp/extract", "/etc/passwd")  = "../../etc/passwd", nil
filepath.Join("/tmp/extract", "../../etc/passwd")  = /etc/passwd   // escapes root

(path is f.Name, an attacker-controlled zip entry name, so it can be any string including an absolute path.)

Ablation confirmation: after removing the barrierModel row, the ZipSlip test reports a new alert on tst.go:27:

| tst.go:23:2:43:2 | range statement[1] | tst.go:23:2:43:2 | range statement[1] | tst.go:27:21:27:48 | call to Join | Unsanitized archive entry, which may contain '..', is used in a $@. | tst.go:27:21:27:48 | call to Join | file system operation |

Steps to reproduce

1. Run the TaintedPath query on the existing test:

   codeql test run go/ql/test/query-tests/Security/CWE-022/TaintedPath.qlref
   

   Observe that line 27 (ReadFile(sanitized_filepath) where sanitized_filepath = filepath.Rel(...)) is not in the alerts.

2. Remove the filepath.Rel barrierModel section from go/ql/lib/ext/path.filepath.model.yml (lines 1-6, the barrierModel block) and re-run. Line 27 is now flagged.

3. The same applies to ZipSlip.qlref / tst.go:27.

Suggested fix

Remove the barrierModel section for filepath.Rel:

# Remove this entire block — filepath.Rel is not a path-injection sanitizer
extensions:
  - addsTo:
      pack: codeql/go-all
      extensible: barrierModel
    data:
      - ["path/filepath", "", False, "Rel", "", "", "ReturnValue", "path-injection", "manual"]

Keep the summaryModel row (taint propagation is correct):

- ["path/filepath", "", False, "Rel", "", "", "Argument[0..1]", "ReturnValue[0]", "taint", "manual"]

After removing the barrier, the TaintedPath.go and tst.go test cases at the locations above should be re-evaluated — they are currently marked GOOD but are path-traversal vulnerabilities (the test expectations will need updating to include the new alerts).

Context

• Tested with CodeQL CLI 2.25.6, repo HEAD a24d222d96 (codeql-cli/latest-577-ga24d222d96).
• filepath.Rel behavior verified by running Go 1.22.12 (see the Rel/Join examples above).
• The barrierModel for Rel was added in commit 7fff3534fa ("Convert 3 barriers for path injection to MaD").
• filepath.Base is documented in CodeQL as "not a sanitizer for path traversal" — Rel should be treated the same way.

[oscarsj]

Thank you for this detailed and well-researched report , this is valid. You're correct that filepath.Rel is not a path-injection sanitizer: it can return relative paths containing .. components that traverse outside the base directory, so it should not be modeled as a barrierModel for path-injection.

We're escalating this to the Go language coverage team to remove the barrier model and update the affected test expectations. The summaryModel (taint propagation) is correct and should remain.

Thanks for including the ablation results and the Go documentation references, that made triaging straightforward.