Skip to content

Commit

Permalink
fix panic due to bad slice indexing in NewUserPassword
Browse files Browse the repository at this point in the history
  • Loading branch information
Tim Cooper authored and Arunangshu Chatterjee committed Sep 15, 2024
1 parent 890bc10 commit 12a61da
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 13 deletions.
10 changes: 5 additions & 5 deletions attribute.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func NewIFID(addr net.HardwareAddr) (Attribute, error) {
// attribute length is invalid, the secret is empty, or the requestAuthenticator
// length is invalid.
func UserPassword(a Attribute, secret, requestAuthenticator []byte) ([]byte, error) {
if len(a) < 16 || len(a) > 128 {
if len(a) < 16 || len(a) > 128 || len(a)%16 != 0 {
return nil, errors.New("invalid attribute length (" + strconv.Itoa(len(a)) + ")")
}
if len(secret) == 0 {
Expand Down Expand Up @@ -204,8 +204,8 @@ func NewUserPassword(plaintext, secret, requestAuthenticator []byte) (Attribute,
hash.Write(requestAuthenticator)
enc = hash.Sum(enc)

for i, b := range plaintext[:16] {
enc[i] ^= b
for i := 0; i < 16 && i < len(plaintext); i++ {
enc[i] ^= plaintext[i]
}

for i := 16; i < len(plaintext); i += 16 {
Expand All @@ -214,8 +214,8 @@ func NewUserPassword(plaintext, secret, requestAuthenticator []byte) (Attribute,
hash.Write(enc[i-16 : i])
enc = hash.Sum(enc)

for j, b := range plaintext[i : i+16] {
enc[i+j] ^= b
for j := 0; j < 16 && i+j < len(plaintext); j++ {
enc[i+j] ^= plaintext[i+j]
}
}

Expand Down
24 changes: 16 additions & 8 deletions attribute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,36 @@ import (

func TestNewUserPassword_length(t *testing.T) {
tbl := []struct {
Password string
Password []byte
EncodedLength int
}{
{"", 16},
{"abc", 16},
{"0123456789abcde", 16},
{"0123456789abcdef", 16},
{"0123456789abcdef0", 16 * 2},
{"0123456789abcdef0123456789abcdef0123456789abcdef", 16 * 3},
{append(make([]byte, 0, 0), ""...), 16},
{append(make([]byte, 0, 15), "abc"...), 16},
{append(make([]byte, 0, 15), "0123456789abcde"...), 16},
{append(make([]byte, 0, 16), "0123456789abcdef"...), 16},
{append(make([]byte, 0, 30), "0123456789abcdef0"...), 16 * 2},
{append(make([]byte, 0, 48), "0123456789abcdefzzzzzzzzzzzzzzzzQQQQQQQQQQQQQQQQ"...), 16 * 3},
}

secret := []byte(`12345`)
ra := []byte(`0123456789abcdef`)

for _, x := range tbl {
attr, err := NewUserPassword([]byte(x.Password), secret, ra)
attr, err := NewUserPassword(x.Password, secret, ra)
if err != nil {
t.Fatal(err)
}
if len(attr) != x.EncodedLength {
t.Fatalf("expected encoded length of %#v = %d, got %d", x.Password, x.EncodedLength, len(attr))
}

decoded, err := UserPassword(attr, secret, ra)
if err != nil {
t.Fatal(err)
}
if !bytes.Equal(decoded, x.Password) {
t.Fatalf("expected roundtrip to succeed")
}
}
}

Expand Down

0 comments on commit 12a61da

Please sign in to comment.