From 11fcec2109b8cd7518fa8dc7c863fa2892b55418 Mon Sep 17 00:00:00 2001 From: Jonathan Rainville Date: Tue, 9 Jul 2024 15:22:59 -0400 Subject: [PATCH] fix(messenger)_: make sure display name is legit before sending (#5496) FIxes https://github.com/status-im/status-desktop/issues/15284 Somehow, the display name could get messed up and when it happened, no message could be received, because we also validate the display name on reception. I think the problem for the particular user happened because the special display name was added before we enforced it, or through the sync. --- protocol/messenger.go | 20 ++++++++++++++++++++ protocol/messenger_backup_handler.go | 26 +++++++++++++++++--------- protocol/messenger_backup_test.go | 26 ++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 9 deletions(-) diff --git a/protocol/messenger.go b/protocol/messenger.go index e9e6a950d..43fa9428e 100644 --- a/protocol/messenger.go +++ b/protocol/messenger.go @@ -29,6 +29,7 @@ import ( "github.com/ethereum/go-ethereum/p2p" "github.com/status-im/status-go/account" "github.com/status-im/status-go/appmetrics" + utils "github.com/status-im/status-go/common" "github.com/status-im/status-go/connection" "github.com/status-im/status-go/contracts" "github.com/status-im/status-go/deprecation" @@ -902,6 +903,25 @@ func (m *Messenger) Start() (*MessengerResponse, error) { return nil, err } + displayName, err := m.settings.DisplayName() + if err != nil { + return nil, err + } + if err := utils.ValidateDisplayName(&displayName); err != nil { + // Somehow a wrong display name was saved. We need to update it so that others accept our messages + pubKey, err := m.settings.GetPublicKey() + if err != nil { + return nil, err + } + replacementDisplayName := pubKey[:12] + m.logger.Warn("unaccepted display name was saved to the setting, reverting to pubkey substring", zap.String("displayName", displayName), zap.String("replacement", replacementDisplayName)) + + if err := m.SetDisplayName(replacementDisplayName); err != nil { + // We do not return the error as we do not want to block the login for it + m.logger.Warn("error setting display name", zap.Error(err)) + } + } + return response, nil } diff --git a/protocol/messenger_backup_handler.go b/protocol/messenger_backup_handler.go index 7480f3a82..d8d028172 100644 --- a/protocol/messenger_backup_handler.go +++ b/protocol/messenger_backup_handler.go @@ -7,6 +7,7 @@ import ( "github.com/golang/protobuf/proto" + utils "github.com/status-im/status-go/common" "github.com/status-im/status-go/images" "github.com/status-im/status-go/multiaccounts/errors" "github.com/status-im/status-go/multiaccounts/settings" @@ -112,16 +113,23 @@ func (m *Messenger) handleBackedUpProfile(message *protobuf.BackedUpProfile, bac Profile: &wakusync.BackedUpProfile{}, } - err := m.SaveSyncDisplayName(message.DisplayName, message.DisplayNameClock) - if err != nil && err != errors.ErrNewClockOlderThanCurrent { - return err - } - - response.SetDisplayName(message.DisplayName) - - // if we already have a newer clock, then we don't need to update the display name - if err == errors.ErrNewClockOlderThanCurrent { + err := utils.ValidateDisplayName(&message.DisplayName) + if err != nil { + // Print a warning and set the display name to the account name, but don't stop the recovery + m.logger.Warn("invalid display name found", zap.Error(err)) response.SetDisplayName(m.account.Name) + } else { + err = m.SaveSyncDisplayName(message.DisplayName, message.DisplayNameClock) + if err != nil && err != errors.ErrNewClockOlderThanCurrent { + return err + } + + response.SetDisplayName(message.DisplayName) + + // if we already have a newer clock, then we don't need to update the display name + if err == errors.ErrNewClockOlderThanCurrent { + response.SetDisplayName(m.account.Name) + } } syncWithBackedUpImages := false diff --git a/protocol/messenger_backup_test.go b/protocol/messenger_backup_test.go index 74ebb9866..d4e03e936 100644 --- a/protocol/messenger_backup_test.go +++ b/protocol/messenger_backup_test.go @@ -7,6 +7,7 @@ import ( "reflect" "testing" + v1protocol "github.com/status-im/status-go/protocol/v1" "github.com/status-im/status-go/protocol/wakusync" "github.com/stretchr/testify/suite" @@ -228,6 +229,31 @@ func (s *MessengerBackupSuite) TestBackupProfile() { s.Require().Equal(clock, lastBackup) } +func (s *MessengerBackupSuite) TestBackupProfileWithInvalidDisplayName() { + // Create bob1 + bob1 := s.m + + state := ReceivedMessageState{ + Response: &MessengerResponse{}, + } + + err := bob1.HandleBackup( + &state, + &protobuf.Backup{ + Profile: &protobuf.BackedUpProfile{ + DisplayName: "bad-display-name_eth", + }, + Clock: 1, + }, + &v1protocol.StatusMessage{}, + ) + // The backup will still work, but the display name will be skipped + s.Require().NoError(err) + storedBob1DisplayName, err := bob1.settings.DisplayName() + s.Require().NoError(err) + s.Require().Equal("", storedBob1DisplayName) +} + func (s *MessengerBackupSuite) TestBackupSettings() { s.T().Skip("flaky test") const (