Skip to content

Commit

Permalink
added sanity checks on user/pass string length when creating user via…
Browse files Browse the repository at this point in the history
… RPC API
  • Loading branch information
swdee committed Mar 13, 2020
1 parent bdfabe9 commit 1aef6e1
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 1 deletion.
12 changes: 11 additions & 1 deletion api/keystore/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,14 @@ import (
jsoncodec "github.com/ava-labs/gecko/utils/json"
)

const (
// maxUserPassLen is the maximum length of the username or password allowed
maxUserPassLen = 1024
)

var (
errEmptyUsername = errors.New("username can't be the empty string")
errEmptyUsername = errors.New("username can't be the empty string")
errUserPassMaxLength = errors.New(fmt.Sprintf("CreateUser call rejected due to username or password exceeding maximum length of %d chars", maxUserPassLen))
)

// KeyValuePair ...
Expand Down Expand Up @@ -114,6 +120,10 @@ func (ks *Keystore) CreateUser(_ *http.Request, args *CreateUserArgs, reply *Cre
ks.lock.Lock()
defer ks.lock.Unlock()

if len(args.Username) > maxUserPassLen || len(args.Password) > maxUserPassLen {
return errUserPassMaxLength
}

ks.log.Verbo("CreateUser called with %s", args.Username)

if args.Username == "" {
Expand Down
52 changes: 52 additions & 0 deletions api/keystore/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ package keystore

import (
"bytes"
"fmt"
"math/rand"
"testing"

"github.com/ava-labs/gecko/database/memdb"
Expand Down Expand Up @@ -56,6 +58,56 @@ func TestServiceCreateUser(t *testing.T) {
}
}

// genStr returns a string of given length
func genStr(n int) string {
b := make([]byte, n)
rand.Read(b)
return fmt.Sprintf("%x", b)[:n]
}

// TestServiceCreateUserArgsChecks generates excessively long usernames or
// passwords to assure the santity checks on string length are not exceeded
func TestServiceCreateUserArgsCheck(t *testing.T) {
ks := Keystore{}
ks.Initialize(logging.NoLog{}, memdb.New())

{
reply := CreateUserReply{}
err := ks.CreateUser(nil, &CreateUserArgs{
Username: genStr(maxUserPassLen + 1),
Password: "shortpass",
}, &reply)

if reply.Success || err != errUserPassMaxLength {
t.Fatal("User was created when it should have been rejected due to too long a Username, err =", err)
}
}

{
reply := CreateUserReply{}
err := ks.CreateUser(nil, &CreateUserArgs{
Username: "shortuser",
Password: genStr(maxUserPassLen + 1),
}, &reply)

if reply.Success || err != errUserPassMaxLength {
t.Fatal("User was created when it should have been rejected due to too long a Password, err =", err)
}
}

{
reply := ListUsersReply{}
if err := ks.ListUsers(nil, &ListUsersArgs{}, &reply); err != nil {
t.Fatal(err)
}

if len(reply.Users) > 0 {
t.Fatalf("A user exists when there should be none")
}
}

}

func TestServiceCreateDuplicate(t *testing.T) {
ks := Keystore{}
ks.Initialize(logging.NoLog{}, memdb.New())
Expand Down

0 comments on commit 1aef6e1

Please sign in to comment.