From e4c1abb5ce6a863219402399a5d208e061bb52f3 Mon Sep 17 00:00:00 2001 From: Mykhailo Prakhov Date: Thu, 28 Mar 2024 16:57:59 +0100 Subject: [PATCH] fix: validate display name on account creation (#4994) --- common/utils.go | 33 +++++++++++++++ images/qr-assets.go | 34 ++++++++-------- protocol/message_validator.go | 3 +- protocol/messenger_handler.go | 5 ++- protocol/messenger_identity.go | 33 +-------------- .../messenger_identity_display_name_test.go | 40 +++++++++++++++++++ protocol/requests/create_account.go | 8 +++- 7 files changed, 105 insertions(+), 51 deletions(-) diff --git a/common/utils.go b/common/utils.go index 43b4bdd95..45b1249f1 100644 --- a/common/utils.go +++ b/common/utils.go @@ -2,11 +2,19 @@ package common import ( "crypto/ecdsa" + "errors" + "regexp" + "strings" "github.com/status-im/status-go/eth-node/crypto" + "github.com/status-im/status-go/protocol/identity/alias" "github.com/status-im/status-go/protocol/protobuf" ) +var ErrInvalidDisplayNameRegExp = errors.New("only letters, numbers, underscores and hyphens allowed") +var ErrInvalidDisplayNameEthSuffix = errors.New(`usernames ending with "eth" are not allowed`) +var ErrInvalidDisplayNameNotAllowed = errors.New("name is not allowed") + func RecoverKey(m *protobuf.ApplicationMetadataMessage) (*ecdsa.PublicKey, error) { if m.Signature == nil { return nil, nil @@ -22,3 +30,28 @@ func RecoverKey(m *protobuf.ApplicationMetadataMessage) (*ecdsa.PublicKey, error return recoveredKey, nil } + +func ValidateDisplayName(displayName *string) error { + name := strings.TrimSpace(*displayName) + *displayName = name + + if name == "" { + return nil + } + + // ^[\\w-\\s]{5,24}$ to allow spaces + if match, _ := regexp.MatchString("^[\\w-\\s]{5,24}$", name); !match { + return ErrInvalidDisplayNameRegExp + } + + // .eth should not happen due to the regexp above, but let's keep it here in case the regexp is changed in the future + if strings.HasSuffix(name, "_eth") || strings.HasSuffix(name, ".eth") || strings.HasSuffix(name, "-eth") { + return ErrInvalidDisplayNameEthSuffix + } + + if alias.IsAlias(name) { + return ErrInvalidDisplayNameNotAllowed + } + + return nil +} diff --git a/images/qr-assets.go b/images/qr-assets.go index ecc698700..3b8954a00 100644 --- a/images/qr-assets.go +++ b/images/qr-assets.go @@ -266,24 +266,26 @@ func AssetNames() []string { // _bindata is a table, holding each asset generator, mapped to its name. var _bindata = map[string]func() (*asset, error){ - "_assets/tests/1.gif": _assetsTests1Gif, - "_assets/tests/2x1.png": _assetsTests2x1Png, - "_assets/tests/spin.gif": _assetsTestsSpinGif, + "_assets/tests/1.gif": _assetsTests1Gif, + "_assets/tests/2x1.png": _assetsTests2x1Png, + "_assets/tests/spin.gif": _assetsTestsSpinGif, "_assets/tests/qr/QRWithLogo.png": _assetsTestsQrQrwithlogoPng, - "_assets/tests/qr/defaultQR.png": _assetsTestsQrDefaultqrPng, - "_assets/tests/qr/status.png": _assetsTestsQrStatusPng, - "_assets/tests/wikipedia.ico": _assetsTestsWikipediaIco, + "_assets/tests/qr/defaultQR.png": _assetsTestsQrDefaultqrPng, + "_assets/tests/qr/status.png": _assetsTestsQrStatusPng, + "_assets/tests/wikipedia.ico": _assetsTestsWikipediaIco, } // AssetDir returns the file names below a certain // directory embedded in the file by go-bindata. // For example if you run go-bindata on data/... and data contains the // following hierarchy: -// data/ -// foo.txt -// img/ -// a.png -// b.png +// +// data/ +// foo.txt +// img/ +// a.png +// b.png +// // then AssetDir("data") would return []string{"foo.txt", "img"} // AssetDir("data/img") would return []string{"a.png", "b.png"} // AssetDir("foo.txt") and AssetDir("notexist") would return an error @@ -314,17 +316,18 @@ type bintree struct { Func func() (*asset, error) Children map[string]*bintree } + var _bintree = &bintree{nil, map[string]*bintree{ "_assets": &bintree{nil, map[string]*bintree{ "tests": &bintree{nil, map[string]*bintree{ - "1.gif": &bintree{_assetsTests1Gif, map[string]*bintree{}}, + "1.gif": &bintree{_assetsTests1Gif, map[string]*bintree{}}, "2x1.png": &bintree{_assetsTests2x1Png, map[string]*bintree{}}, "qr": &bintree{nil, map[string]*bintree{ "QRWithLogo.png": &bintree{_assetsTestsQrQrwithlogoPng, map[string]*bintree{}}, - "defaultQR.png": &bintree{_assetsTestsQrDefaultqrPng, map[string]*bintree{}}, - "status.png": &bintree{_assetsTestsQrStatusPng, map[string]*bintree{}}, + "defaultQR.png": &bintree{_assetsTestsQrDefaultqrPng, map[string]*bintree{}}, + "status.png": &bintree{_assetsTestsQrStatusPng, map[string]*bintree{}}, }}, - "spin.gif": &bintree{_assetsTestsSpinGif, map[string]*bintree{}}, + "spin.gif": &bintree{_assetsTestsSpinGif, map[string]*bintree{}}, "wikipedia.ico": &bintree{_assetsTestsWikipediaIco, map[string]*bintree{}}, }}, }}, @@ -376,4 +379,3 @@ func _filePath(dir, name string) string { cannonicalName := strings.Replace(name, "\\", "/", -1) return filepath.Join(append([]string{dir}, strings.Split(cannonicalName, "/")...)...) } - diff --git a/protocol/message_validator.go b/protocol/message_validator.go index cace2bd64..147b00aa5 100644 --- a/protocol/message_validator.go +++ b/protocol/message_validator.go @@ -6,6 +6,7 @@ import ( "strconv" "strings" + utils "github.com/status-im/status-go/common" "github.com/status-im/status-go/protocol/protobuf" "github.com/status-im/status-go/protocol/v1" ) @@ -343,7 +344,7 @@ func ValidateReceivedChatMessage(message *protobuf.ChatMessage, whisperTimestamp return errors.New("mutual state event system message content type not allowed") } - if err := ValidateDisplayName(&message.DisplayName); err != nil { + if err := utils.ValidateDisplayName(&message.DisplayName); err != nil { return err } diff --git a/protocol/messenger_handler.go b/protocol/messenger_handler.go index 2f3c26f6c..b759d20a1 100644 --- a/protocol/messenger_handler.go +++ b/protocol/messenger_handler.go @@ -17,6 +17,7 @@ import ( "github.com/google/uuid" + utils "github.com/status-im/status-go/common" "github.com/status-im/status-go/eth-node/crypto" "github.com/status-im/status-go/eth-node/types" "github.com/status-im/status-go/images" @@ -1212,7 +1213,7 @@ func (m *Messenger) HandleContactUpdate(state *ReceivedMessageState, message *pr return ErrMessageNotAllowed } - if err = ValidateDisplayName(&message.DisplayName); err != nil { + if err = utils.ValidateDisplayName(&message.DisplayName); err != nil { return err } @@ -3023,7 +3024,7 @@ func (m *Messenger) HandleChatIdentity(state *ReceivedMessageState, ci *protobuf } if clockChanged { - if err = ValidateDisplayName(&ci.DisplayName); err != nil { + if err = utils.ValidateDisplayName(&ci.DisplayName); err != nil { return err } diff --git a/protocol/messenger_identity.go b/protocol/messenger_identity.go index f9f2d3842..eb370a50c 100644 --- a/protocol/messenger_identity.go +++ b/protocol/messenger_identity.go @@ -4,15 +4,14 @@ import ( "context" "errors" "fmt" - "regexp" "runtime" "strings" + utils "github.com/status-im/status-go/common" "github.com/status-im/status-go/multiaccounts/settings" sociallinkssettings "github.com/status-im/status-go/multiaccounts/settings_social_links" "github.com/status-im/status-go/protocol/encryption/multidevice" "github.com/status-im/status-go/protocol/identity" - "github.com/status-im/status-go/protocol/identity/alias" "github.com/status-im/status-go/server" ) @@ -21,38 +20,10 @@ const ( maxSocialLinkTextLength = 24 ) -var ErrInvalidDisplayNameRegExp = errors.New("only letters, numbers, underscores and hyphens allowed") -var ErrInvalidDisplayNameEthSuffix = errors.New(`usernames ending with "eth" are not allowed`) -var ErrInvalidDisplayNameNotAllowed = errors.New("name is not allowed") var ErrInvalidBioLength = errors.New("invalid bio length") var ErrInvalidSocialLinkTextLength = errors.New("invalid social link text length") var ErrDisplayNameDupeOfCommunityMember = errors.New("display name duplicates on of community members") -func ValidateDisplayName(displayName *string) error { - name := strings.TrimSpace(*displayName) - *displayName = name - - if name == "" { - return nil - } - - // ^[\\w-\\s]{5,24}$ to allow spaces - if match, _ := regexp.MatchString("^[\\w-\\s]{5,24}$", name); !match { - return ErrInvalidDisplayNameRegExp - } - - // .eth should not happen due to the regexp above, but let's keep it here in case the regexp is changed in the future - if strings.HasSuffix(name, "_eth") || strings.HasSuffix(name, ".eth") || strings.HasSuffix(name, "-eth") { - return ErrInvalidDisplayNameEthSuffix - } - - if alias.IsAlias(name) { - return ErrInvalidDisplayNameNotAllowed - } - - return nil -} - func (m *Messenger) SetDisplayName(displayName string) error { currDisplayName, err := m.settings.DisplayName() if err != nil { @@ -63,7 +34,7 @@ func (m *Messenger) SetDisplayName(displayName string) error { return nil // Do nothing } - if err = ValidateDisplayName(&displayName); err != nil { + if err = utils.ValidateDisplayName(&displayName); err != nil { return err } diff --git a/protocol/messenger_identity_display_name_test.go b/protocol/messenger_identity_display_name_test.go index 2a135a8af..b0e79ddb0 100644 --- a/protocol/messenger_identity_display_name_test.go +++ b/protocol/messenger_identity_display_name_test.go @@ -5,6 +5,7 @@ import ( "errors" "testing" + utils "github.com/status-im/status-go/common" "github.com/status-im/status-go/multiaccounts/accounts" "github.com/status-im/status-go/protocol/encryption/multidevice" "github.com/status-im/status-go/protocol/tt" @@ -182,3 +183,42 @@ func (s *MessengerProfileDisplayNameHandlerSuite) TestDisplayNameSync() { s.Require().NoError(err) s.Require().Equal(testDisplayName, dbProfileKp.Name) } + +func (s *MessengerProfileDisplayNameHandlerSuite) TestDisplayNameRestrictions() { + // check display name for the created instance + displayName, err := s.m.settings.DisplayName() + s.Require().NoError(err) + s.Require().Equal(DefaultProfileDisplayName, displayName) + + // add profile keypair + profileKp := accounts.GetProfileKeypairForTest(true, false, false) + profileKp.KeyUID = s.m.account.KeyUID + profileKp.Name = DefaultProfileDisplayName + profileKp.Accounts[0].KeyUID = s.m.account.KeyUID + + err = s.m.settings.SaveOrUpdateKeypair(profileKp) + s.Require().NoError(err) + + setInvalidName := func(invalidName string, expectedErr error) { + err = s.m.SetDisplayName(invalidName) + s.Require().ErrorIs(err, expectedErr) + } + + setInvalidName("test.eth", utils.ErrInvalidDisplayNameRegExp) + setInvalidName("test-eth", utils.ErrInvalidDisplayNameEthSuffix) + setInvalidName("test_eth", utils.ErrInvalidDisplayNameEthSuffix) + + setInvalidName("dot.not", utils.ErrInvalidDisplayNameRegExp) + setInvalidName("t", utils.ErrInvalidDisplayNameRegExp) + setInvalidName("tt", utils.ErrInvalidDisplayNameRegExp) + setInvalidName("ttt", utils.ErrInvalidDisplayNameRegExp) + setInvalidName("tttt", utils.ErrInvalidDisplayNameRegExp) + setInvalidName("name is bigger than 24 symb", utils.ErrInvalidDisplayNameRegExp) + + err = s.m.SetDisplayName("name with space") + s.Require().NoError(err) + displayName, err = s.m.settings.DisplayName() + s.Require().NoError(err) + s.Require().Equal("name with space", displayName) + +} diff --git a/protocol/requests/create_account.go b/protocol/requests/create_account.go index ffc8d7b00..d90118d32 100644 --- a/protocol/requests/create_account.go +++ b/protocol/requests/create_account.go @@ -1,7 +1,9 @@ package requests import ( - "errors" + "github.com/pkg/errors" + + utils "github.com/status-im/status-go/common" ) var ErrCreateAccountInvalidDisplayName = errors.New("create-account: invalid display name") @@ -87,6 +89,10 @@ func (c *CreateAccount) Validate(validation *CreateAccountValidation) error { return ErrCreateAccountInvalidDisplayName } + if err := utils.ValidateDisplayName(&c.DisplayName); err != nil { + return errors.Wrap(ErrCreateAccountInvalidDisplayName, err.Error()) + } + if len(c.Password) == 0 { return ErrCreateAccountInvalidPassword }