code-423n4 / 2021-09-sushitrident-2-findings

0 stars 0 forks source link

`ConcentratedLiquidityPoolManager.sol#reclaimIncentive()` Unsafe implementation allows malicious users to steal yield #29

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

WatchPug

Vulnerability details

The reclaimIncentive() function allows users who added incentives before to withdraw unclaimed rewards.

However, the current implementation did not manage the state correctly, incentive.rewardsUnclaimed is not updated after the token transfer, which allows the user to call reclaimIncentive() again and get incentive.token up to the amount of rewardsUnclaimed.

When the incentive.token is set to token0 or token1, this can be used for stealing yields from other users.

https://github.com/sushiswap/trident/blob/c405f3402a1ed336244053f8186742d2da5975e9/contracts/pool/concentrated/ConcentratedLiquidityPoolManager.sol#L59-L60

/// @dev Withdraws any unclaimed incentive rewards.
function reclaimIncentive(
    IConcentratedLiquidityPool pool,
    uint256 incentiveId,
    uint256 amount,
    address receiver,
    bool unwrapBento
) public {
    Incentive storage incentive = incentives[pool][incentiveId];
    require(incentive.owner == msg.sender, "NOT_OWNER");
    require(incentive.expiry < block.timestamp, "EXPIRED");
    require(incentive.rewardsUnclaimed >= amount, "ALREADY_CLAIMED");
    _transfer(incentive.token, address(this), receiver, amount, unwrapBento);
    emit ReclaimIncentive(pool, incentiveId);
}

Recommended Mitigation Steps

Consider removing the amount parameter and always withdraw all the unclaimed rewards to the owner, and delete the incentive.

sarangparikh22 commented 3 years ago

Duplicate of #37