Fee credits for social token holders are not correctly tracked

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2024-01-curves/blob/main/contracts/Curves.sol#L218-L508 https://github.com/code-423n4/2024-01-curves/blob/main/contracts/FeeSplitter.sol#L73-L117

Vulnerability details

Preliminary note

The goal of the FeeSplitter's contract is to distribute fees to current holders of the social tokens generating the correspondig fees. Furthermore, fees must not be distributed to withdrawn social tokens.

However, ownership of social tokens is not correctly tracked in order to correctly assign fee credits. Following cases are not appropriately handled: 1) transfer of a social token, 2) withraw of social token, 3) deposit of social token, 4) buy of a social token, and 5) sell of a social token

In all of these cases the Curves contract misses to appropriately inform the FeeSplitter contract in order that FeeSplitter can appropriately update the user variables userFeeOffset and unclaimedFees.

Impact

Entire Ether balance in the FeeSplitter can be stolen.

Proof of Concept

For example, consider the following attack vector to drain the entire available Ether balance from the FeeSplitter contract:

1) Attacker buys with his address #1 amount X of social tokens using buyCurvesToken in Curves. -> address #1 has X social tokens of type A and actual userFeeOffset is set 2) Attacker transfer X social tokens to his address #2 using transferCurvesToken in Curves -> address #1 has 0 social tokens of type A and actual userFeeOffset is set -> address #2 has X social tokens and userFeeOffset is still '0' 3) Attacker calls claimFees in FeeSplitter with address #2 -> address #2 has X social tokens and userFeeOffset is set and user has already some unClaimedFees since userFeeOffset was 0 in: uint256 owed = (data.cumulativeFeePerToken - data.userFeeOffset[account]) * balance; 4) Attacker repeats steps 2) and 3) with different other addresses #3, #4,... until all available Ether balance has been stolen.

Tools Used

Manual review.

Recommended Mitigation Steps

Rework on the FeeSplitter and Curves smart contract to ensure that in all cases the ownership of social tokens is correctly recorded. In particular, ensure that the following user variables are always correctly updated:

• userFeeOffset
• unclaimedFees

When reworking the contracts with regards to this issus consider also the reported issues with regards to "setFeeRedistributor", which could be tackled by either not allowing or to also track the token balances in the FeeSplitter contract.

Following the rework of the FeeSplitter and Curve smart contracts, conduct a mitigation security review.

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #39

[c4-judge]

alcueca marked the issue as satisfactory