encoding/base64 Decode panics if given too short dst

[natefinch]

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

go 1.18

Does this issue reproduce with the latest release?

yes

What did you do?

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

What set this off is that I accidentally set the wrong value for an environment variable, which got sent into base64.Decode, which panicked. That seemed weird because I would not expect the standard library to panic... especially since Decode returns an error anyway, so I would expect it to just... return an error.

I did some googling, and saw some people say "oh, just use DecodedLen to figure out if your buffer is big enough" ... so I was like, ok, I'll do that. Except that DecodedLen gives the maximum size your buffer needs to be, not the actual size. So, some encoded strings will appear to need a larger buffer than they actually require. Our fake key used in tests is a good example: AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA= ... decodedLen gives you 33, but it actually decodes to 32. If you only have a 32 byte buffer... there's no way to check beforehand that it'll fit. You just have to try it. And then the standard library might panic, you can't know.

To properly write code that simply tries to decode a string that should be a 32 byte value and not panic... I had to write something like this (which seems kinda ridiculous)

// decodeKey converts a base64 string into a 32 byte key
func decodeKey(encodedKey string) ([32]byte, error) {
	var result [32]byte
	// 44 bytes is the maximum length of a base64-encoded 32 byte key
	if len(encodedKey) > 44 {
		return result, fmt.Errorf("invalid length for encoded key, should be 44 or less, was: %d", len(encodedKey))
	}
	// If you try to decode into a buffer that is too small, the standard
	// library will panic. Unfortunately, there's no way to know exactly what
	// size the buffer needs to be. A 32 byte key will never encode to larger
	// than 44 bytes, so we check that above. But a 44 byte encoded string could
	// decode into up to 33 bytes. So we allocate a buffer that is 33 bytes so
	// we can avoid the panic. We then check the size of what was written below,
	// and error out if it's not 32 bytes.
	var decoded [33]byte
	n, err := base64.StdEncoding.Decode(decoded[:], []byte(encodedKey))
	if err != nil {
		return [32]byte{}, err
	}
	if n != 32 {
		return result, fmt.Errorf("invalid length for key, should be 32 bytes, but was %d", n)
	}
	copy(result[:], decoded[:])
	return result, nil
}

What did you expect to see?

Preferably, just return an error if the slice is too small.

At the very least give me a better panic message than an index out of range error.

What did you see instead?

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

goroutine 1 [running]:
encoding/base64.(*Encoding).decodeQuantum(0xc000074df8?, {0xc000074e9e?, 0x40?, 0x0?}, {0xc000106000?, 0x40?, 0xc000106000?}, 0xc000106000?)
	/usr/local/go-faketime/src/encoding/base64/base64.go:360 +0x2b3
encoding/base64.(*Encoding).Decode(0xc000100000, {0xc000074e80, 0x20, 0x20}, {0xc000106000, 0x34, 0x40})
	/usr/local/go-faketime/src/encoding/base64/base64.go:528 +0x565

[seankhliao]

Duplicate of #50117

[randall77]

Probably could workaround using #53693 if it is accepted.

[CleanCut]

@seankhliao #50117 was closed on the grounds that it's fine to panic if you pass in nil. This isn't about nil, it's about a buffer with insufficient capacity.

If we (@natefinch, me, whoever) put together a PR to return an error on insufficient capacity, would that get a fair chance at review?

[natefinch]

In my opinion, the "invalid data is ok to panic on" is not applicable here. You can't know your buffer is invalid until you try (inside certain parameters). Is that 44 byte base64 string going to fit in my 32 byte buffer? Maybe. Maybe not.

If the answer is to make a bigger buffer, then it defeats the purpose of being able to pass in my own buffer. That's the reason you use this API, so you can pass in a buffer to fill.

[seankhliao]

it's easy to check, the last character of input must be = (padding)

[natefinch]

@seankhliao then why doesn't the base64 package check that for me? What's the point of having a package for the format if I have to know the nitty gritty details of the encoding rules in order to keep the package from panicking?