False positive: go/zipslip when `filepath.IsLocal` is already used
- Lenguaje dominante
- CodeQL
- Estrellas
- 10.1k
- Forks
- 2.1k
- Merge medio
- 2 d 15 h
- PR fusionados (30 d)
- 141
Descripción
go/zipslip was detected, but the case was already protected by `filepath.IsLocal`.
Code example:
```go
r := tar.NewReader(bytes.NewReader(data))
for {
hdr, err := r.Next()
if err != nil {
if errors.Is(err, io.EOF) {
break // End of archive.
}
return fmt.Errorf("failed to read next tar entry: %v", err)
}
name := hdr.Name
if !filepath.IsLocal(name) {
continue
}
if hdr.FileInfo().IsDir() {
continue
}
// ... Make files/dirs based on name.
```
https://github.com/microsoft/go-infra/blob/7d114900fe9286d0fa400d02c6c5034b439d955b/gitcmd/gitcmd.go#L89-L125
https://github.com/microsoft/go-infra/security/code-scanning/4
> [IsLocal](https://pkg.go.dev/path/filepath#IsLocal) (added in go1.20) reports whether path, using lexical analysis only, has all of these properties:
>
> * is within the subtree rooted at the directory in which path is evaluated
> * is not an absolute path
> * is not empty
> * on Windows, is not a reserved name such as "NUL"
>
> If IsLocal(path) returns true, then Join(base, path) will always produce a path contained within base and Clean(path) will always produce an unrooted path with no ".." path elements.
>
> IsLocal is a purely lexical operation. In particular, it does not account for the effect of any symbolic links that may exist in the filesystem.
https://pkg.go.dev/archive/tar#Reader.Next mentions `IsLocal` as the way Go may automatically prevent zipslip:
> If Next encounters a non-local name (as defined by [filepath.IsLocal](https://pkg.go.dev/path/filepath#IsLocal)) and the GODEBUG environment variable contains `tarinsecurepath=0`, Next returns the header with an [ErrInsecurePath](https://pkg.go.dev/archive/tar#ErrInsecurePath) error. A future version of Go may introduce this behavior by default. Programs that want to accept non-local names can ignore the [ErrInsecurePath](https://pkg.go.dev/archive/tar#ErrInsecurePath) error and use the returned header.
---
I found an existing issue about go/zipslip, but it's about looking inside a func, not `IsLocal`:
* https://github.com/github/codeql/issues/17573
Guía de contribución
Línea de trabajo
Comienza con las líneas 89-125 enlazadas de gitcmd.go y la alerta de go/zipslip; después compara la guardia de filepath.IsLocal con la lógica de detección de la consulta. Revisa issue 17573 para obtener contexto relacionado. Se considera terminado cuando este caso protegido ya no se reporte sin suprimir hallazgos genuinos de zipslip.
Escrito por el modelo de indexación a partir del texto del issue.
Evaluación
- Stack tecnológico
- go
- Área
- security
- Tipo de issue
- Error
- Dificultad
- 4/5
- Tiempo estimado
- 3-5 días
- Estado de actividad
- Estancado
- Claridad
- Bastante claro
- Aptitud para principiantes
- 38/100