sherlock-audit / 2024-06-boost-aa-wallet-judging

3 stars 1 forks source link

denzi_ - Malicious User can grief by frontrunning `ERC20Incentive::clawback()` function by calling `ERC20Incentive::claim()` to make the initial clawback call revert. #403

Open sherlock-admin3 opened 2 months ago

sherlock-admin3 commented 2 months ago

denzi_

Medium

Malicious User can grief by frontrunning ERC20Incentive::clawback() function by calling ERC20Incentive::claim() to make the initial clawback call revert.

Summary

ERC20Incentive allows 2 kinds of strategies, Raffles and Pools. This submission discusses how a malicious user can grief the owner of the incentive and the contract by frontrunning the clawback function call made by the owner when the strategy is Raffle.

The clawback function allows the owner to reclaim the assets in the incentive contract. For Raffles, the reclaim amount should be the full amount and no claims (entries) in the raffle should have been made.

Vulnerability Detail

The code for the clawback function is the following

/// @inheritdoc AIncentive
    function clawback(bytes calldata data_) external override onlyOwner returns (bool) {
        ClawbackPayload memory claim_ = abi.decode(data_, (ClawbackPayload));
        (uint256 amount) = abi.decode(claim_.data, (uint256));

        if (strategy == Strategy.RAFFLE) {
            // Ensure the amount is the full reward and there are no raffle entries, then reset the limit
            if (amount != reward || claims > 0) revert BoostError.ClaimFailed(msg.sender, abi.encode(claim_)); // @audit-issue a user can frontrun this to increment the claims and make clawback revert

            limit = 0; 
        } else {
            // Ensure the amount is a multiple of the reward and reduce the max claims accordingly
            if (amount % reward != 0) revert BoostError.ClaimFailed(msg.sender, abi.encode(claim_));
            limit -= amount / reward;
        }

        // Transfer the tokens back to the intended recipient
        asset.safeTransfer(claim_.target, amount);
        emit Claimed(claim_.target, abi.encodePacked(asset, claim_.target, amount));

        return true;
    }

The code requires amount != reward and claims > 0 for the callback to execute when the incentive is a Raffle strategy. Here claims represent the number of entries in the Raffle.

Consider the following situation

Bob has created a Raffle strategy incentive with a high limit and low reward amount, hence due to other better options no one has joined the raffle. Bob decides to call clawback and recover the amount he deposited into the incentive and close it (the clawback functions sets limit to 0 so no claims can be made). Bob's tx can seen on the mempool.

Now a malicious user watches this tx and decides to grief bob. The malicious user generates/has an already created signature for this moment. They call claim with a higher gas fees. This transaction will increment the claims variable through the claim() function in the incentive.

Now bob's tx will get executed which will revert due to claims > 0 (claim will be 1 here) and bob will not be able to recover their amount.

This causes bob to either wait out for more people to join and continue with the raffle (against his wishes), most likely if an incentive is unpopular, users will not participate in his boost causing him to just draw or keep the amount in the incentive forever.

Impact

Owner can be frontran and their funds will not be recovered.

Code Snippet

ERC20Incentive::clawback()

Tool used

Manual Review

Recommendation

For raffles, Use a pause/unpause modifier so the owner can set the raffle to pause and only clawback can be called during the pause. This allows them to safely recover their amount without other interference.