Project

Vulnerability Details

Let's start with the flow of

1. First, funds enter the contract through MembershipERC1155::sendProfit():

contracts/dao/tokens/MembershipERC1155.sol#L189-L200

1. When users want to claim, they call MembershipERC1155::claimProfit():

contracts/dao/tokens/MembershipERC1155.sol#L142-L150

1. The MembershipERC1155::saveProfit() function is called, which itself calls MembershipERC1155::getUnsaved():

This function calculates shares based on token balances at the timestamp profits were claimed, not when they were funded.

The problem arises when users can update their token balances between the time profits are distributed and claimed, leading to an unfair distribution of profits.

Consider the following scenario:

• Let a DAO totalSupply be 2, two tokens of the lowest tier.

• At timestamp t: 2 honest users -Alice and Bob- own 1 token each.

• At timestamp t+1: The DAO gets funds by calling MembershipERC1155::sendProfit by an amount of 300 ERC20 tokens.

• By the current MembershipERC1155::claimProfit logic, each user is elligible to 150 token. This should hold at any timestamp t+n for that particular fund allocation to maintain fairness.

• At timestamp t+2: The innocent Alice calls MembershipERC1155::joinDAO and gets a second token of the same tier. Bob didn't claim his profit yet. Notice that claiming profits is not enforced at the time of funds allocation i.e of calling sendProfit()

• At timestamp t+3: Alice is elligible to claim 200 tokens now, while Bob is elligible to claim 100 tokens only.

• Bob effectively lost 50 tokens (33% of his elligible profits) while Alice got 50 tokens for free.

To recap, the two root causes are:

• Profits are calculated using claim timestamp rather than funding timestamp and,

• Profit claiming does not necessarily happen at the time of profit funding.

Impact

The system is flawed and unfair. The described scenario could be scaled to any configuration of DAO and users.

The unfair distribution of profits, even in case of honest behavior of all users, causes direct fund losses to user.

Tools Used

Manual review.

Recommendations

• Consider adding a mapping that records ERC1155 token balances at the time of profit funding, not at the time of claim.

• Consider enforcing distribution of profits directly at sendProfit which will enforce token balances at the time of calling it to avoid such timing and unfairness issues.

Note: this issue is different than Cyfrin's 7.2.1. Notice that even by fixing the issue in our report, an attacker could still frontrun a sendProfit function to get a higher share of DAO funding.