github / github/codeql

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

Aberta
#22,214 3 comentários 0 reações 0 responsáveis Ver no GitHub
question
Linguagem predominante
CodeQL
Estrelas
10.1k
Forks
2.1k
Merge médio
2d 15h
PRs com merge (30d)
141

Descrição

**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**

```go
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

Guia de contribuição

Abrir o guia de contribuição

Direção de pesquisa

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.

Escrita pelo modelo de indexação a partir do texto da issue.

Avaliação

Stack de tecnologia
go
Domínio
security
Tipo de issue
Bug
Dificuldade
4/5
Tempo estimado
3-5 dias
Status de atividade
Pouca atividade
Clareza
Razoavelmente clara
Facilidade para iniciantes
50/100

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.