hpke: slice bounds out of range in shortKEM.UnmarshalBinaryPrivateKey

[emersion]

When passing a byte slice of size > 66 bytes to shortKEM.UnmarshalBinaryPrivateKey when using the DHKEM(P-521, HKDF-SHA512) KEM, the following panic is triggered:

panic: runtime error: slice bounds out of range [-1:]

goroutine 3014 [running]:
panic({0x78c980?, 0xc00050a090?})
	/usr/lib/go/src/runtime/panic.go:770 +0x132
github.com/cloudflare/circl/hpke.shortKEM.UnmarshalBinaryPrivateKey({{{0x12, {0x7aa461, 0x19}, 0x7}, {0x820178, 0xc000016240}}, {0x81fe98, 0x9ad8e0}}, {0xc0002800a0, 0x43, ...})
	/home/simon/src/circl/hpke/shortkem.go:87 +0x35b

[armfazh]

This is a bug, the code shouldn't panic, but instead it should return an error.
in #489 , code enforces to pass slices of the exact size for unmarshaling keys.

[bwesterb]

Not sure whether it's a bug to panic when the caller breaks the function contract. Not opposed to returning an error instead though.

[emersion]

I don't see where this function contract is defined? Nothing in https://pkg.go.dev/github.com/cloudflare/[email protected]/kem#Scheme

IMHO, panic'ing would be OK if it was documented and if it used an explicit check with a proper error message (instead of an out of bounds error).

However, please note that other functions with documented panics don't return an error (so don't really have a choice). UnmarshalBinaryPrivateKey does return an error. Moreover, "Unmarshal" is a kind of parsing step, and parsing deals with user-provided input. Panic'ing on bad user-provided input (such as fed from a network protocol) isn't super nice.