Skip to content

Go: Fix incorrect path injection sanitizer FilePath.Rel#22150

Open
owen-mc wants to merge 1 commit into
github:mainfrom
owen-mc:go/remove-incorrect-sanitizer
Open

Go: Fix incorrect path injection sanitizer FilePath.Rel#22150
owen-mc wants to merge 1 commit into
github:mainfrom
owen-mc:go/remove-incorrect-sanitizer

Conversation

@owen-mc

@owen-mc owen-mc commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Reported in #22143. This mistake has been in the code for over 6 years, since the first commit of the CodeQL library for Go!

@owen-mc owen-mc marked this pull request as ready for review July 9, 2026 20:22
@owen-mc owen-mc requested a review from a team as a code owner July 9, 2026 20:22
@owen-mc owen-mc requested review from a team, Copilot and oscarsj July 9, 2026 20:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a long-standing incorrect Go path-injection/ZipSlip sanitization model for path/filepath.Rel, which previously suppressed legitimate path-traversal findings by treating Rel (and an err == nil pattern around it) as a sanitizer.

Changes:

  • Remove the barrierModel entry that treated filepath.Rel’s return value as a path-injection barrier.
  • Remove the TaintedPath customization that treated err == nil from filepath.Rel(base, path) as a containment/sanitizer guard.
  • Update the CWE-022 test cases and .expected outputs to reflect the newly-detected flows, and add a changenote documenting the analysis impact.
Show a summary per file
File Description
go/ql/lib/ext/path.filepath.model.yml Removes the incorrect barrier model for filepath.Rel while keeping taint propagation.
go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll Drops the Rel-based “containment” sanitizer guard that caused false negatives.
go/ql/test/query-tests/Security/CWE-022/TaintedPath.go Updates inline expectations to mark filepath.Rel usage as unsafe where appropriate.
go/ql/test/query-tests/Security/CWE-022/TaintedPath.expected Updates expected results to include the newly surfaced go/path-injection alert paths.
go/ql/test/query-tests/Security/CWE-022/tst.go Updates ZipSlip test annotations to treat Rel-based containment as unsafe.
go/ql/test/query-tests/Security/CWE-022/ZipSlip.expected Updates expected results to include the newly surfaced go/zipslip path(s).
go/ql/lib/change-notes/2026-07-09-remove-incorrect-path-sanitizer.md Documents the modeling fix and the expected increase in results.

Review details

  • Files reviewed: 7/7 changed files
  • Comments generated: 0
  • Review effort level: Low

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