From f702b54d08e463fa8b29a69d5b8c0ddd214f8b40 Mon Sep 17 00:00:00 2001
From: Pascal Precht <445106+0x-r4bbit@users.noreply.github.com>
Date: Mon, 19 Jun 2023 12:33:48 +0200
Subject: [PATCH] fix: don't check permissions if revealed accounts aren't
 legit

This seems to be a bug that was introduced when two features, admin
permissions and "always reveal wallet accounts" where merged.

We need to make sure we **first** check the revealed accounts and only
**then** do we perform permission checks on them. Otherwise we can run
into scenarios where fake addresses are used and users will be accepted
to the community.
---
 protocol/communities/manager.go | 64 ++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/protocol/communities/manager.go b/protocol/communities/manager.go
index bcf204c3d..84fc8f604 100644
--- a/protocol/communities/manager.go
+++ b/protocol/communities/manager.go
@@ -1783,6 +1783,38 @@ func (m *Manager) HandleCommunityRequestToJoin(signer *ecdsa.PublicKey, request
 		return nil, err
 	}
 
+	if len(request.RevealedAccounts) > 0 {
+		// verify if revealed addresses indeed belong to requester
+		for _, revealedAccount := range request.RevealedAccounts {
+			recoverParams := account.RecoverParams{
+				Message:   types.EncodeHex(crypto.Keccak256(crypto.CompressPubkey(signer), community.ID(), requestToJoin.ID)),
+				Signature: types.EncodeHex(revealedAccount.Signature),
+			}
+
+			recovered, err := m.accountsManager.Recover(recoverParams)
+			if err != nil {
+				return nil, err
+			}
+			if recovered.Hex() != revealedAccount.Address {
+				// if ownership of only one wallet address cannot be verified,
+				// we mark the request as cancelled and stop
+				err = m.markRequestToJoinAsCanceled(signer, community)
+				if err != nil {
+					return nil, err
+				}
+				requestToJoin.State = RequestToJoinStateDeclined
+				return requestToJoin, nil
+			}
+		}
+
+		// Save revealed addresses + signatures so they can later be added
+		// to the community member list when the request is accepted
+		err = m.persistence.SaveRequestToJoinRevealedAddresses(requestToJoin)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	becomeAdminPermissions := community.TokenPermissionsByType(protobuf.CommunityTokenPermission_BECOME_ADMIN)
 	becomeMemberPermissions := community.TokenPermissionsByType(protobuf.CommunityTokenPermission_BECOME_MEMBER)
 
@@ -1847,38 +1879,6 @@ func (m *Manager) HandleCommunityRequestToJoin(signer *ecdsa.PublicKey, request
 		requestToJoin.State = RequestToJoinStateAccepted
 	}
 
-	// Save revealed addresses + signatures so they can later be added
-	// to the community member list when the request is accepted
-	if len(request.RevealedAccounts) > 0 {
-		// verify if revealed addresses indeed belong to requester
-		for _, revealedAccount := range request.RevealedAccounts {
-			recoverParams := account.RecoverParams{
-				Message:   types.EncodeHex(crypto.Keccak256(crypto.CompressPubkey(signer), community.ID(), requestToJoin.ID)),
-				Signature: types.EncodeHex(revealedAccount.Signature),
-			}
-
-			recovered, err := m.accountsManager.Recover(recoverParams)
-			if err != nil {
-				return nil, err
-			}
-			if recovered.Hex() != revealedAccount.Address {
-				// if ownership of only one wallet address cannot be verified,
-				// we mark the request as cancelled and stop
-				err = m.markRequestToJoinAsCanceled(signer, community)
-				if err != nil {
-					return nil, err
-				}
-				requestToJoin.State = RequestToJoinStateDeclined
-				return requestToJoin, nil
-			}
-		}
-
-		err = m.persistence.SaveRequestToJoinRevealedAddresses(requestToJoin)
-		if err != nil {
-			return nil, err
-		}
-	}
-
 	return requestToJoin, nil
 }