Maia DAO Ecosystem - giovannidisiena's results

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/gauges/BaseV2Gauge.sol#L144-L152 https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/rewards/depots/MultiRewardsDepot.sol#L38-L40

Vulnerability details

Impact

When calling BaseV2Gauge::removeBribeFlywheel, the given bribe flywheel is deleted from the active bribe flywheels mapping in BaseV2Gauge; however, the associated bribe asset which was previously added to multiRewardsDepot in a call to MultiRewardsDepot::addAsset via BaseV2Gauge::addBribeFlywheel is not removed from multiRewardsDepot.

function addBribeFlywheel(FlywheelCore bribeFlywheel) external onlyOwner {
    /// @dev Can't add existing flywheel (active or not)
    if (added[bribeFlywheel]) revert FlywheelAlreadyAdded();

    address flyWheelRewards = address(bribeFlywheel.flywheelRewards());
    FlywheelBribeRewards(flyWheelRewards).setRewardsDepot(multiRewardsDepot);

    multiRewardsDepot.addAsset(flyWheelRewards, bribeFlywheel.rewardToken());
    bribeFlywheels.push(bribeFlywheel);
    isActive[bribeFlywheel] = true;
    added[bribeFlywheel] = true;

    emit AddedBribeFlywheel(bribeFlywheel);
}

function removeBribeFlywheel(FlywheelCore bribeFlywheel) external onlyOwner {
    /// @dev Can only remove active flywheels
    if (!isActive[bribeFlywheel]) revert FlywheelNotActive();

    /// @dev This is permanent; can't be re-added
    delete isActive[bribeFlywheel]; // @audit - asset not removed on depot though so deprecated flywheel can still call `getRewards`

    emit RemoveBribeFlywheel(bribeFlywheel);
}

This means that, whilst the bribe flywheel is deprecated, its associated reward contract can still call MultiRewardsDepot::getRewards which will transfer the entire contract balance of the reward token to the removed bribe flywheel's reward contract.

function getRewards() external override(RewardsDepot, IMultiRewardsDepot) onlyFlywheelRewards returns (uint256) {
    return transferRewards(_assets[msg.sender], msg.sender);
}

function transferRewards(address _asset, address _rewardsContract) internal returns (uint256 balance) {
    balance = _asset.balanceOf(address(this));
    _asset.safeTransfer(_rewardsContract, balance);
}

Tools Used

Manual review

Recommended Mitigation Steps

Ensure MultiRewardsDepot::removeAsset is called when removing a bribe flywheel from the gauge if it is not intended to have the reward contracts of deprecated bribe flywheels get rewards.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/talos/libraries/PoolActions.sol#L82-L85

Vulnerability details

Impact

Zero values for amount0Min and amount1Min exposes strategies to infinite slippage in calls to PoolActions::rerange which opens up opportunities for sandwich attacks. Additionally, use of block.timestamp as the deadline means that a given transaction will be valid whenever the validator decides to include it.

(tokenId, liquidity, amount0, amount1) = nonfungiblePositionManager.mint(
    INonfungiblePositionManager.MintParams({
        token0: address(actionParams.token0),
        token1: address(actionParams.token1),
        fee: poolFee,
        tickLower: tickLower,
        tickUpper: tickUpper,
        amount0Desired: balance0,
        amount1Desired: balance1,
        amount0Min: 0, // @audit - infinite slippage
        amount1Min: 0,
        recipient: address(this),
        deadline: block.timestamp // @audit - validator can include whenever they choose
    })
);

Tools Used

Manual review

Recommended Mitigation Steps

Replace zero min values with values closer to the desired output amounts to minimise slippage and consider adding a more appropriate deadline if the current behaviour is not desired.

Assessed type

Uniswap