github / github/codeql

Go: zip-slip FP / missed a zip-slip guard in argoproj/argo-cd

Open
#17,573 3 comments 0 reactions 0 assignees View on GitHub
Dominant language
CodeQL
Stars
10.1k
Forks
2.1k
Avg merge
2d 15h
Merged PRs (30d)
141

Description

https://github.com/github/codeql/blob/590e93d8edec4d7216935ed4425a7ab77b3b2f34/go/ql/src/Security/CWE-022/ZipSlip.ql#L22-L23

Here's my fork's report:
https://github.com/check-spelling-sandbox/argo-cd/security/code-scanning/4

---

Arbitrary file access during archive extraction ("Zip Slip")

Code snippet
[util/io/files/tar.go:75](https://github.com/check-spelling-sandbox/argo-cd/blob/4014cc8b040f55dc698295d658cf0eb780ea7203/util/io/files/tar.go#L75-L75)
```go
tr := tar.NewReader(lr)

for {
header, err := tr.Next()
```
> Unsanitized archive entry, which may contain '..', is used in a .

---

Here's the accused flow:

Arbitrary file access during archive extraction ("Zip Slip")
Step 1 ... := ...[0]
Source
[util/io/files/tar.go:75](https://github.com/check-spelling-sandbox/argo-cd/blob/4014cc8b040f55dc698295d658cf0eb780ea7203/util/io/files/tar.go#L75-L75)
```go
tr := tar.NewReader(lr)

for {
header, err := tr.Next()
```
> Unsanitized archive entry, which may contain '..', is used in a .
> Unsanitized archive entry, which may contain '..', is used in a .
> Unsanitized archive entry, which may contain '..', is used in a .
```go
if err != nil {
if err == io.EOF {
break
```
Step 2 selection of Name
[util/io/files/tar.go:86](https://github.com/check-spelling-sandbox/argo-cd/blob/4014cc8b040f55dc698295d658cf0eb780ea7203/util/io/files/tar.go#L86-L86)
```go
continue
}

target := filepath.Join(dstPath, header.Name)
```
> [!NOTE]
> There _is_ a check for zip-slip right here in the form of [Inbound](https://github.com/argoproj/argo-cd/blob/b8249567ae1afe657f3d2f235dc3724880c91370/util/io/files/util.go#L75-L94):
```go
// Sanity check to protect against zip-slip
if !Inbound(target, dstPath) {
return fmt.Errorf("illegal filepath in archive: %s", target)
```
Step 3 call to Join
[util/io/files/tar.go:86](https://github.com/check-spelling-sandbox/argo-cd/blob/4014cc8b040f55dc698295d658cf0eb780ea7203/util/io/files/tar.go#L86-L86)
```go
continue
}

target := filepath.Join(dstPath, header.Name)
// Sanity check to protect against zip-slip
if !Inbound(target, dstPath) {
return fmt.Errorf("illegal filepath in archive: %s", target)
```
Step 4 target
Sink
[util/io/files/tar.go:98](https://github.com/check-spelling-sandbox/argo-cd/blob/4014cc8b040f55dc698295d658cf0eb780ea7203/util/io/files/tar.go#L98-L98)
```go
if preserveFileMode {
mode = os.FileMode(header.Mode)
}
err := os.MkdirAll(target, mode)
if err != nil {
return fmt.Errorf("error creating nested folders: %w", err)
}
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.