Abracadabra Mimswap - grearlake's results

Lines of code

https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/staking/LockingMultiRewards.sol#L528-#L534

Vulnerability details

Vulnerability details

Whenever any operation with the given user as a beneficiary is performed, this user's rewards are checkpointed via function _updateRewardsGlobal(), which calculates the reward checkpoint by a call to function _rewardPerToken

function _rewardPerToken(address rewardToken, uint256 lastTimeRewardApplicable_, uint256 totalSupply_) public view returns (uint256) {
    if (totalSupply_ == 0) {
        return _rewardData[rewardToken].rewardPerTokenStored;
    }

    uint256 timeElapsed = lastTimeRewardApplicable_ - _rewardData[rewardToken].lastUpdateTime;
    uint256 pendingRewardsPerToken = (timeElapsed * _rewardData[rewardToken].rewardRate * 1e18) / totalSupply_;

    return _rewardData[rewardToken].rewardPerTokenStored + pendingRewardsPerToken;
}

rewardRate is set in notifyRewardAmount function as below:

    reward.rewardRate = amount / _remainingRewardTime;

It is clearly that pendingRewardsPerToken can be equal to 0. Consider this mock scenario:

• timeElapsed = 12 seconds (equal to the time of a block)
• _rewardData[rewardToken].rewardRate = 2
• totalSupply_ = 1e22
• _rewardData[rewardToken].rewardPerTokenStored = x

So the result of the function is x + 12 * 2 * 1e18 / 1e22 Which is equal to x + 24e18 / 1e22 = x due to rounding down

Which is incorrect as the calculation does not account for the rewards accumulated since the last checkpoint.

Attacker can griefing attack by continously calling stake or lock function with dust amount, since they only revert when amount = 0:

function _stakeFor(address account, uint256 amount, bool lock_) internal {
    if (amount == 0) {
        revert ErrZeroAmount();  // <---
    }

function lock(uint256 amount) public whenNotPaused {
    if (amount == 0) {
        revert ErrZeroAmount();  // <--
    }

Impact

Incorrect checkpoint value being stored for the beneficiary, which will affect future reward calculations for this beneficiary.

Tools Used

Manual review

Recommended Mitigation Steps

Use validation checks to ensure that the required calculation doesn't round down to zero:

   require(pendingRewardsPerToken >= totalSupply_, "rounding down to 0")

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L470-#L510 https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L244-#L265

Vulnerability details

Vulnerability details

Because MagicLP is forked from DODO, we will check DODO documentation to understand this contract. From DODO documentation, the "I" is the "i" value in here and it is directly related with the output amount a trader will receive when selling a quote/base token:

Adjusting the value of "I" directly by calling setParameters() function will influence the price. This can be exploited by a MEV bot, simply by trading just before the setParameters() function and exiting right after the price change. The profit gained from this operation essentially represents potential losses for the liquidity providers who supplied liquidity to the pool.

Impact

Since the price will change, the MEV bot can simply sandwich the tx and get profit.

Another note on this is that even though the adjustPrice called by onlyImplementationOwner without getting frontrunned, it still creates a big price difference which requires immediate arbitrages. Usually these type of parameter changes that impacts the trades are setted by time via ramping to mitigate the unfair advantages that it can occur during the price update.

Tools Used

Manual review

Recommended Mitigation Steps

Acknowledge the issue and use private RPC's to eliminate front-running or slowly ramp up the "I" so that the arbitrage opportunity is fair

Assessed type

MEV

Lines of code

https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastOnboarding.sol#L132

Vulnerability details

Vulnerability details

From the contest documentation:

People who deposited unlocked into the LLE can withdraw at any time during and after

BlastOnboarding.withdraw() function:

function withdraw(address token, uint256 amount) external whenNotPaused onlySupportedTokens(token) {
    balances[msg.sender][token].unlocked -= amount;
    balances[msg.sender][token].total -= amount;
    totals[token].unlocked -= amount;
    totals[token].total -= amount;

    token.safeTransfer(msg.sender, amount);

    emit LogWithdraw(msg.sender, token, amount);
}

However, due to the whenNotPaused modifier in the BlastOnboarding.withdraw() function, users who have deposit unlocked will not be able to withdraw their funds. If the protocol is paused, it will fail, causing the transaction to revert and preventing users from withdrawing their funds.

Tools Used

Manual review

Recommended Mitigation Steps

Remove whenNotPaused modifier from withdraw() function:

 -  function withdraw(address token, uint256 amount) external whenNotPaused onlySupportedTokens(token)
 +  function withdraw(address token, uint256 amount) external onlySupportedTokens(token)

Assessed type

Other