Slice bounds panic in radius.NewUserPassword

[aaronbee]

This code in NewUserPassword will panic if cap(plaintext) < 16:

	for i, b := range plaintext[:16] {
		enc[i] ^= b
	}

There is similar code in UserPassword as well.

[u873838]

Please provide a reproducible example.

[aaronbee]

This program reproduces the panic:

package main

import (
	"layeh.com/radius"
	"layeh.com/radius/rfc2865"
)

func main() {
	packet := radius.New(radius.CodeAccessRequest, []byte("secret"))
	password := make([]byte, 0, 16) // capacity is 16
	password = append(password, "password"...)
	rfc2865.UserPassword_Set(packet, password) // no panic

	packet = radius.New(radius.CodeAccessRequest, []byte("secret"))
	password = make([]byte, 0, 15) // capacity less than 16
	password = append(password, "password"...)
	rfc2865.UserPassword_Set(packet, password) // panic
}

With the following backtrace:

panic: runtime error: slice bounds out of range [:16] with capacity 15

goroutine 1 [running]:
layeh.com/radius.NewUserPassword({0xc00006bf11, 0x8, 0xf}, {0xc000014146, 0x6, 0x6}, {0xc000082089, 0x10, 0x10})
	layeh.com/radius@v0.0.0-20200407174255-3e43fd4ead92/attribute.go:207 +0x3e5
layeh.com/radius/rfc2865.UserPassword_Set(0xc000082080, {0xc00006bf11?, 0x6?, 0x6?})
	layeh.com/radius@v0.0.0-20200407174255-3e43fd4ead92/rfc2865/generated.go:238 +0x54
main.main()
	radius_repro/main.go:17 +0xca
exit status 2

EDIT: Fixed code block above to include panic message

[aaronbee]

Just an interesting note, we have been using this package for a while and only recently ran into this when testing with Go's development branch. A new optimization to avoid copies when converting between string -> []byte started causing this to hit. Our code actually uses rfc2865.UserPassword_SetString, which takes in a string, converts to []byte and then calls radius.NewUserPassword. It seems without the new optimization this results in a new []byte to always be created with at least 16 capacity. With the optimization the allocation of the []byte is skipped and hence the capacity is whatever size the string is, which can be less than 16.

In other words, this optimization in Go unearths a long-standing bug present in NewUserPassword that it doesn't check that it's argument has enough capacity before reslicing.

[u873838]

Thank you. The issue should be fixed in 6c2c615.