Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/mod/sumdb/dirhash: panic observed traversing tmp dir #57269

Closed
simar7 opened this issue Dec 12, 2022 · 4 comments · Fixed by aquasecurity/trivy#3606
Closed

x/mod/sumdb/dirhash: panic observed traversing tmp dir #57269

simar7 opened this issue Dec 12, 2022 · 4 comments · Fixed by aquasecurity/trivy#3606
Labels
FrozenDueToAge help wanted modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@simar7
Copy link

simar7 commented Dec 12, 2022

What version of Go are you using (go version)?

$ go version
go version go1.19.1 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/simar/Library/Caches/go-build"
GOENV="/Users/simar/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/simar/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/simar/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.19.1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/tmp/foo-bar-program/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/46/5653r61560j_p5gx3b46l01w0000gn/T/go-build1816768646=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

package main

import (
        "fmt"
        "golang.org/x/mod/sumdb/dirhash"
        "log"
)

func main() {
        got, err := dirhash.DirFiles("/tmp/", "")
        if err != nil {
                log.Printf(err.Error())
        }

        fmt.Println(got)
}

What did you expect to see?

No panic

What did you see instead?

Panic

go run main.go 
panic: runtime error: slice bounds out of range [5:4]

goroutine 1 [running]:
golang.org/x/mod/sumdb/dirhash.DirFiles.func1({0x10c47c2, 0x4}, {0x10e79c8?, 0xc000100680?}, {0x0?, 0x0?})
        /Users/simar/go/pkg/mod/golang.org/x/mod@v0.7.0/sumdb/dirhash/hash.go:96 +0x22b
path/filepath.walk({0x10c47c2, 0x4}, {0x10e79c8, 0xc000100680}, 0xc000074ee0)
        /usr/local/go/src/path/filepath/path.go:433 +0x123
path/filepath.Walk({0x10c47c2, 0x4}, 0xc000074ee0)
        /usr/local/go/src/path/filepath/path.go:520 +0x6c
golang.org/x/mod/sumdb/dirhash.DirFiles({0x10c47c2?, 0xc0000061a0?}, {0x0, 0x0})
        /Users/simar/go/pkg/mod/golang.org/x/mod@v0.7.0/sumdb/dirhash/hash.go:87 +0x9b
main.main()
        /tmp/foo-bar-program/main.go:10 +0x2d
exit status 2

Few extra observations:

  1. This seems to only happen when /tmp/ is passed as a param. The perms on the /tmp dir on macOS:
 ls -lrth /tmp
lrwxr-xr-x@ 1 root  admin    11B Mar  6  2020 /tmp -> private/tmp
  1. It does not seem to be a permission issue as passing another directory that is not owned by the user that's running the program seems to be fine (not panic)
@gopherbot gopherbot added this to the Unreleased milestone Dec 12, 2022
@bcmills
Copy link
Contributor

bcmills commented Dec 12, 2022

This occurs any time the argument to DirFiles is the path to a valid non-directory.
For example, it also reproduces if you pass the path to a regular file: https://go.dev/play/p/MrisxXZftxy

The bug is here:
https://cs.opensource.google/go/x/mod/+/master:sumdb/dirhash/hash.go;l=96;drc=346a37af5599be02f125bd8bc0a5e1d33db21ddc

The error is the assumption that len(file) > len(dir), which does not hold for the argument file itself. If the argument is a directory, the function returns before that line is reached.

The fix is probably to return an explicit error if the argument dir is not a directory.

@bcmills bcmills added help wanted NeedsFix The path to resolution is known, but the work has not been done. modules labels Dec 12, 2022
@simar7
Copy link
Author

simar7 commented Dec 12, 2022

Thanks for the insight. I’ll try to put a fix together.

simar7 added a commit to simar7/mod that referenced this issue Dec 12, 2022
@simar7
sumdb/dirhash: fix a panic when argument is not a directory
26b037e 
This patch fixes a case where a path to a non directory
can cause DirHash func to panic.

Fixes: golang/go#57269

Signed-off-by: Simar <simar@linux.com>
@simar7 simar7 mentioned this issue Dec 12, 2022
Closed
@simar7
Copy link
Author

simar7 commented Dec 12, 2022

I've created a PR to address this here: golang/mod#16

simar7 added a commit to simar7/mod that referenced this issue Dec 13, 2022
@simar7
sumdb/dirhash: fix a panic when argument is not a directory
9fbbc03 
This patch fixes a case where a path to a non directory
can cause DirHash func to panic.

Fixes: golang/go#57269.
simar7 added a commit to simar7/mod that referenced this issue Dec 13, 2022
@simar7
sumdb/dirhash: fix a panic when argument is not a directory
e3d0940 
This patch fixes a case where a path to a non directory
can cause DirHash func to panic.

Fixes: golang/go#57269.
simar7 added a commit to simar7/mod that referenced this issue Jan 9, 2023
@simar7
sumdb/dirhash: fix a panic when argument is not a directory
a17ca1a 
This patch fixes a case where a path to a non directory
can cause DirHash func to panic.

Fixes: golang/go#57269.
@gopherbot
Copy link

Change https://go.dev/cl/457075 mentions this issue: sumdb/dirhash: fix a panic when argument is not a directory

@knqyf263 knqyf263 mentioned this issue Feb 13, 2023
Merged
6 tasks
@golang golang locked and limited conversation to collaborators Jan 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unreleased
3 participants
@simar7 @bcmills @gopherbot