x/crypto/sha3: bytepad function is not safe

[paocalvi]

Go version

1.23 (all)

Output of go env in your module/workspace:

set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\...\AppData\Local\go-build
set GOENV=C:\Users\...\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\calvip\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\...\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=c:\go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=c:\go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.22.3
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=NUL
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=C:\Users\.....\AppData\Local\Temp\go-build939558769=/tmp/go-build -gno-record-gcc-switches

What did you do?

Static analysis of the x/crypto/sha3/shake.go function

What did you see happen?

The code

func bytepad(input []byte, w int) []byte {
	// leftEncode always returns max 9 bytes
	buf := make([]byte, 0, 9+len(input)+w)
	buf = append(buf, leftEncode(uint64(w))...)
	buf = append(buf, input...)
	padlen := w - (len(buf) % w)
	return append(buf, make([]byte, padlen)...)
}

and in particular

padlen := w - (len(buf) % w)

is not working properly when len(buf) % w == 0, causing an undesired padding.

The function appear to work only if the length of the input + length of the letfencoding is not a multiple of w already.

What did you expect to see?

leftEncode([]uint8("12345678"),10) // leftencode should be 2 bytelong that summed with 8 is 10
shoud return a 10 bytes result, not a 20 bytes result.

[gabyhelp]

Related Issues and Documentation

• x/crypto/sha3: cSHAKE initialization misbehaves for extremely (unrealistically) large `N` or `S` #66232

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[ianlancetaylor]

CC @golang/security

[magical]

@paocalvi's analysis seems right to me.

Here's a reproducer for cSHAKE-128:

	s := []byte(strings.Repeat("x", 168-7))
	fmt.Printf("S=%s\n", s)
	cs := sha3.NewCShake128(nil, s)
	h := cs.Sum(nil)
	fmt.Printf("%x\n", h)
	// Output: 430d3ebae1528304465f3b6f2ed34a7b931af804afe97d0e2a2796abf5725281
	// Expected output: 2cf20c4b26c9ee7751eaa273368e616c868e7275178634e1ecdbac80d4cab5f4

https://go.dev/play/p/RQRAsLybuBg

The 161-byte customization string ensures that the length of the bytepadded initial block will have a length equal to cSHAKE-128's rate (168 bytes).

Expected output taken from this random js implementation. Someone should probably verify it against the reference implementation, though.

[FiloSottile]

Very unfortunate that neither our tests nor Wycheproof caught this.

[gopherbot]

Change https://go.dev/cl/616576 mentions this issue: sha3: fix padding for long cSHAKE parameters