c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L349-L357 https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L397-L405

Vulnerability details

Impact

Users can delay Prime token minting to earn extra interest and a larger portion of rewards for themselves.

Proof of Concept

The protocol features an issue() function designed for minting both revocable and irrevocable tokens to users via timelock proposals.

However, a potential issue arises due to a check within the _mint() function:

if (tokens[user].exists) revert IneligibleToClaim();

This check causes the entire transaction to fail if any user already possesses a token. This creates an opportunity for a user who meets the criteria of stakedAt[user] + 90 days < block.timestamp and is on the list of intended recipients for token minting to front-run the transaction and force it to revert by claiming their token.

This scenario benefits such a user because of the way the reward system operates. Reward distribution per user is determined by the sumOfMembersScore specific to the token. When more users are added, their scores contribute to the sumOfMembersScore, resulting in a smaller share of rewards per block for each user.

Consider a situation with an ongoing reward distribution cycle. In such a case, a user can simply follow the steps described above to claim additional interest from the contract before other users are included, maximising their gains.

The central assumption here is that issue() is executed via a governance proposal, preventing immediate re-execution in the same block due to the required mutation of proposal calldata.

Consider the following PoC demonstrating the issue at hand: https://gist.github.com/CrisCodesCrap/426f1e1f5b70b370c09ee9cceb8d5e01

Tools Used

Manual Review, Hardhat

Recommended Mitigation Steps

You could either adopt a pull-over-push approach or implement a check to determine if each user is eligible to call claim() independently before processing them.

for (uint256 i = 0; i < users.length; ) {
    if (stakedAt[user] + 90 days < block.timestamp) continue;

    _mint(false, users[i]);
    _initializeMarkets(users[i]);
    delete stakedAt[users[i]];

    unchecked {
        i++;
    }
}