Members can undermine the intent of the `invite_member` extrinsic by inviting themselves to get free tokens

[viniul]

Summary

The membership pallet grants every user who bought a membership the possibility to invite a limited number of other members. Upon inviting other AccountIds, those account ids are granted an initial invitation_balance, which can only be used to pay Transaction fees. Right now, it is possible to transfer this invitation_balance to one's own account, by inviting one's own account id. It is also possible to invite AccountId which already have a membership, which undermines the intent of the invitation system.

To mitigate this issue, only allow invites to controller_ids which are not already tied to any membership.

Issue Details

The invite_member extrinsic runtime-modules/membership/src/lib.rs takes as an argument the InviteMembershipParameters, which includes the controller_account . In the extrinsic, an initial invitation_balance is transferred from the membership working group balance is to the controller_account. This invitation_balance can only be used to pay transaction fees:

pub fn invite_member(origin,
params: InviteMembershipParameters)
            [...]
            // Create default balance for the invited member.
            
            let _ = balances::Module::::deposit_creating(
                &params.controller_account,
                default_invitation_balance
            );

            // Lock invitation balance. Allow only transaction payments.
            T::InvitedMemberStakingHandler::lock_with_reasons(
                &params.controller_account,
                default_invitation_balance,
                WithdrawReasons::except(WithdrawReason::TransactionPayment)
            );
            [...]

The intent of this invitation balance is to draw more members towards the Joystream platform. However, the invite_member extrinsic allows members to invite a Member with a controller_account that actually already has a membership. This controller_account would be granted the initial_invitation balance, undermining the intent of the extrinsic.

In the current configuration, upon buying a membership, 5 invites are granted. Those 5 invites will add up to a total balance of 500 tokens being deposited.

To mitigate this issue, only allow invites to controller_ids which are not already tied to any membership.

Risk

This can be abused in two ways:

1. An attacker could use all their invites on themselves. That is, they could call invite_member and with params.controller_account == origin, for instance.
2. Two attackers who already have membership could invite each other and get granted 5*initial_invitation_balance

Since invited members are not granted any further invites, this is issue is only low severity.

Mitigation

In the invite_member extrinsic, enforce that members can only invite accounts with a controller_account that does not control a membership yet. Another way would be to not pay any balance to controller_account in case the controller_account does already have existing membership.

[shamil-gadelshin]

The issue could be exploited even with the "not a member controller account" check because of the controller account change mechanism. We are introducing the check for the existing invitation lock to allow doing the "trick" only once.

[viniul]

Thanks for pointing this out @shamil-gadelshin! Could you elaborate on the check you are introducing? Where and how exactly would this check be introduced?

[shamil-gadelshin]

Could you elaborate on the check you are introducing? Where and how exactly would this check be introduced?

There is a mistake in the invite_member extrinsic. We didn't check for existing locks. It allowed inviting a member multiple times with printing money to the same account with no effective locks except the first one because locks were overlapping. The coming PR introduces such a check: https://github.com/Joystream/joystream/pull/2187