x/crypto/blowfish: panic: runtime error: index out of range [0] with length 0

catenacyber commented 1 year ago

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

$ go version
go version go1.19 linux/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=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/root/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/root/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/root/.go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/root/.go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/src/ngolo-fuzzing/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 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2481516251=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Run https://go.dev/play/p/QMNkWgpIag1

What did you expect to see?

The program finishing and printing Hello

What did you see instead?

panic: runtime error: index out of range [0] with length 0

goroutine 1 [running]:
golang.org/x/crypto/blowfish.ExpandKey({0xc0000a4f58?, 0xc0000a4f70?, 0x405359?}, 0x0?)
    /tmp/gopath936803848/pkg/mod/golang.org/x/crypto@v0.3.0/blowfish/block.go:34 +0x285
main.main()
    /tmp/sandbox3374802724/prog.go:13 +0x5b

Program exited.

Found by https://github.com/catenacyber/ngolo-fuzzing

Is this relevant ? (Should not panic and do nothing if the slice is empty)

cherrymui commented 1 year ago

cc @golang/security

rolandshoemaker commented 1 year ago

x/crypto/blowfish is a painfully low level package which will panic in a handful of ways if you hold it incorrectly, it would ideally be an internal package only used for bcrypt, but for backwards compatibility reasons that is not really possible.

Given there are no error returns, and often doing nothing may actually be more dangerous than panicking, the only real fix is probably to introduce purposeful panics which catch these issues before they happen, 🤷.

catenacyber commented 1 year ago

Thanks for the answer.

An explicit panic with a string instead of index out of range is fine for me :-)

golang / go