Go: Why is DotDotCheck modeled as a complete path-injection barrier?
まだ誰も着手していません。
評価
調査の方向性
TaintedPathCustomizations.qll の DotDotCheck (106-122 行目) から始め、次に TaintedPath.go の 31 行目にある GOOD ケースと、Go のパスインジェクションクエリのテストを調べます。false ブランチが SanitizerGuardAsSanitizer を通じて isBarrier になる仕組みを確認し、意図された動作を filepath.Base のガイダンスと比較します。完了とは、モデルと影響を受けるテストの期待値が、サポートされているパストラバーサル対策を一貫して反映していることを意味します。
索引モデルが issue の本文から書いたものです。
説明
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
- 主要言語
- CodeQL
- スター
- 10.1k
- フォーク
- 2.1k
- 平均マージ
- 2日 10時間
- マージ済み PR(30日)
- 134
コントリビューションガイド
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
github/codeql のほかの issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 84/100
-
C#: cs/simplifiable-boolean-expression false positive on Nullable<bool> compared with a literal オープン
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
-
false-positive
難易度 2/5 1〜3時間 初心者へのやさしさ 70/100
-
false-positive
難易度 3/5 1〜2日 初心者へのやさしさ 68/100
似ている issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 88/100
sipyourdrink-ltd/bernstein#6191 ·
-
security severity:low track:open-source
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
-
[addtool] DeepZero オープン
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
gwen001/offsectools_www#2055 ·
-
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
WalletConnect/actions#112 ·