False positive: go/zipslip when filepath.IsLocal is already used #20043
Description
go/zipslip was detected, but the case was already protected by filepath.IsLocal.
Code example:
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/security/code-scanning/4
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) and the GODEBUG environment variable contains
tarinsecurepath=0, Next returns the header with an 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 error and use the returned header.
I found an existing issue about go/zipslip, but it's about looking inside a func, not IsLocal: