Maia DAO Ecosystem - lukejohn's results

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/ulysses-amm/UlyssesPool.sol#L223-L292

Vulnerability details

Impact

Detailed description of the impact of this finding. UlyssesPool.setWeight() fails to assign the correct bandwidth to bandwidthStateList[bandwidthStateList.length - 1].bandwidth. The main problem is that it assigns additional leftOverBandwidth to bandwidthStateList[bandwidthStateList.length - 1].bandwidth, which is too large than it is supposed to be.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

UlyssesPool.setWeight() allows a user to set a new weight for an existing poolId. There are two cases:

1. oldTotalWeights < newTotalWeights, then some bandwith will be redistributed from the remaining bandwithStateLists to bandwidthStateList[poolIndex].bandwidth.

2. 1. oldTotalWeights > newTotalWeights, then some bandwith will be redistributed from bandwidthStateList[poolIndex].bandwidth to the remaining bandwithStateLists.

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/ulysses-amm/UlyssesPool.sol#L223-L292

However, L280-281 assigns additional leftOverBandwidth to bandwidthStateList[bandwidthStateList.length - 1].bandwidth, which is too large than it is supposed to be. This is because leftOverBandwidth is the total amount that will be distributed to the remaining bandwithStateLists, not just to bandwidthStateList[bandwidthStateList.length - 1].bandwidth.

Tools Used

VSCode

Recommended Mitigation Steps

Treat the case for bandwidthStateList[bandwidthStateList.length - 1].bandwidth like the remaining case of bandwidthStateList[i].bandwidth.

Assessed type

Math

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/ulysses-amm/UlyssesPool.sol#L223-L305

Vulnerability details

Impact

Detailed description of the impact of this finding. setWeight() gets the logic wrong for the distribution of leftOverBandwidth. The main problem is that it gets the case of oldTotalWeights > newTotalWeights and the case of oldTotalWeights < newTotalWeights confused.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

UlyssesPool.setWeight() allows a user to set a new weight for an existing poolId. There are two cases:

1. oldTotalWeights < newTotalWeights, then some bandwith will be redistributed from the remaining bandwithStateLists to bandwidthStateList[poolIndex].bandwidth.

2. 1. oldTotalWeights > newTotalWeights, then some bandwith will be redistributed from bandwidthStateList[poolIndex].bandwidth to the remaining bandwithStateLists.

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/ulysses-amm/UlyssesPool.sol#L223-L292

However, the implementation gets the logic wrong: it treats the first case as the second case and vice versa. For example, when oldTotalWeights > newTotalWeights, it uses the following logic:

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/ulysses-amm/UlyssesPool.sol#L252C13-L262

Since oldTotalWeights > newTotalWeights, bandwidthStateList[i].bandwidth > oldBandwidth, as a result L260 will always revert due to underflow error. Therefore, the function setWeight() will revert as well.

Tools Used

VSCode

Recommended Mitigation Steps

We need to exchange the logic of the two cases.

 /// @inheritdoc IUlyssesPool
    function setWeight(uint256 poolId, uint8 weight) external nonReentrant onlyOwner {
        if (weight == 0) revert InvalidWeight();

        uint256 poolIndex = destinations[poolId];

        if (poolIndex == 0) revert NotUlyssesLP();

        uint256 oldRebalancingFee;

        for (uint256 i = 1; i < bandwidthStateList.length; i++) {
            uint256 targetBandwidth = totalSupply.mulDiv(bandwidthStateList[i].weight, totalWeights);

            oldRebalancingFee += _calculateRebalancingFee(bandwidthStateList[i].bandwidth, targetBandwidth, false);
        }

        uint256 oldTotalWeights = totalWeights;
        uint256 weightsWithoutPool = oldTotalWeights - bandwidthStateList[poolIndex].weight;
        uint256 newTotalWeights = weightsWithoutPool + weight;
        totalWeights = newTotalWeights;

        if (totalWeights > MAX_TOTAL_WEIGHT || oldTotalWeights == newTotalWeights) {
            revert InvalidWeight();
        }

        uint256 leftOverBandwidth;

        BandwidthState storage poolState = bandwidthStateList[poolIndex];
        poolState.weight = weight;

-        if (oldTotalWeights > newTotalWeights) {
+        if (oldTotalWeights < newTotalWeights) {

            for (uint256 i = 1; i < bandwidthStateList.length;) {
                if (i != poolIndex) {
                    uint256 oldBandwidth = bandwidthStateList[i].bandwidth;
                    if (oldBandwidth > 0) {
                        bandwidthStateList[i].bandwidth =
                            oldBandwidth.mulDivUp(oldTotalWeights, newTotalWeights).toUint248();

                        leftOverBandwidth += oldBandwidth - bandwidthStateList[i].bandwidth;
                    }
                }

                unchecked {
                    ++i;
                }
            }

            poolState.bandwidth += leftOverBandwidth.toUint248();
        } else {
            uint256 oldBandwidth = poolState.bandwidth;
            if (oldBandwidth > 0) {
                poolState.bandwidth = oldBandwidth.mulDivUp(oldTotalWeights, newTotalWeights).toUint248();

                leftOverBandwidth += oldBandwidth - poolState.bandwidth;
            }

            for (uint256 i = 1; i < bandwidthStateList.length;) {
                if (i != poolIndex) {
                    if (i == bandwidthStateList.length - 1) {
                        bandwidthStateList[i].bandwidth += leftOverBandwidth.toUint248();
                    } else if (leftOverBandwidth > 0) {
                        bandwidthStateList[i].bandwidth +=
                            leftOverBandwidth.mulDiv(bandwidthStateList[i].weight, weightsWithoutPool).toUint248();
                    }
                }

                unchecked {
                    ++i;
                }
            }
        }

        uint256 newRebalancingFee;

        for (uint256 i = 1; i < bandwidthStateList.length; i++) {
            uint256 targetBandwidth = totalSupply.mulDiv(bandwidthStateList[i].weight, totalWeights);

            newRebalancingFee += _calculateRebalancingFee(bandwidthStateList[i].bandwidth, targetBandwidth, false);
        }

        if (oldRebalancingFee < newRebalancingFee) {
            asset.safeTransferFrom(msg.sender, address(this), newRebalancingFee - oldRebalancingFee);
        }
    }

Assessed type

Math

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/talos/boost-aggregator/BoostAggregator.sol#L159-L162

Vulnerability details

Impact

Detailed description of the impact of this finding. BoostAggregator.withdrawProtocolFees() fails to check the return value of uniswapV3Staker.claimReward(), as a result, some protocol fee might be lost. This is because the return value uniswapV3Staker.claimReward() is the actual claimed protocol fee, which might be less than protocolRewards. When this happens, protocolRewards should not be cleared to zero, but subtracted by the real amount instead.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

BoostAggregator.withdrawProtocolFees() allows one to withdraw the protocol fee.

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/talos/boost-aggregator/BoostAggregator.sol#L159-L162

However, it fails to check the return value of uniswapV3Staker.claimReward(), which is the actual amount of rewards that have been claimed. Note that this actual amount might be less than protocolRewards, see the implementation of uniswapV3Staker.claimReward():

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/uni-v3-staker/UniswapV3Staker.sol#L262-L274

Therefore, one needs to check the return value of uniswapV3Staker.claimReward() and then update protocolRewards appropriately, see below for the correction.

Tools Used

VSCode

Recommended Mitigation Steps

function withdrawProtocolFees(address to) external onlyOwner {
-        uniswapV3Staker.claimReward(to, protocolRewards);
+        uint256 actualClaimed = uniswapV3Staker.claimReward(to, protocolRewards);
-        delete protocolRewards;
+        protocolRewards -= actualClaimed;
    }

Assessed type

Math

Judge has assessed an item in Issue #198 as 2 risk. The relevant finding follows:

QA1. UlyssesPool.maxRedeem() needs to consider the protocol fees.

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/rewards/base/FlywheelCore.sol#L124-L134

Vulnerability details

Impact

Detailed description of the impact of this finding. FlywheelCore.setFlywheelRewards() allows the owner to set a new (or initialize) FlyWheelRewards contract. However, initially, flywheelRewards has a zero address. As a result, an attacker can send some reward tokens to the zero address, then FlywheelCore.setFlywheelRewards() will always fail since the rewardToken.safeTransferFrom() will always fail since one cannot transfer any reward token out of a zero address.

Similar DOS attack can be launched to BribesFactory.createBribeFlywheel(). The attacker just needs to front-run the transaction and send little bribeTokens to the zero address, then flywheel.setFlywheelRewards() will always fail, and thus BribesFactory.createBribeFlywheel() will always fail also.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

FlywheelCore.setFlywheelRewards() allows the owner to set a new FlyWheelRewards.

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/rewards/base/FlywheelCore.sol#L124-L134

Each FlyWheelCore is associated with a unique FlyWheelReward contract, at the same time, each FlyWheelReward is associated with a unique FlyWheelCore. To establish such a 1-to-1 relationship, what needs to be done is 1) to create the FlyWheelCore contract with a zero address for the FlyWheelReward contract, 2) to create the FlyWheelReward contract with the the previously created FlyWheelCore as the argument for its contractor; 3) Finally, call FlywheelCore.setFlywheelRewards() to set the FlyWheelRewards contract for the FlyWheelCore. See how this is done in the BribesFactory.createBribeFlywheel() function as an example.

The problem is that rewardToken.safeTransferFrom() will be called to transfer the reward tokens from the former FlyWheelRewards contract to the new FlyWheelRewards contract. When the former FlyWheelRewards address is zero and when the oldBlance is not equal to zero, such transfer will always fail since nobody can use rewardToken.safeTransferFrom() to transfer reward tokens out of the zero address.

To exploit such vulnerability, an attacker can then send some reward tokens to the zero address, as a result, rewardToken.safeTransferFrom() will be called in function FlywheelCore.setFlywheelRewards() but it will always fail. This effectively is a DOS attack.

The following POC code confirms my finding - pay attention to function testSetFlywheelRewards():

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "forge-std/Test.sol";
import "../src/maia/tokens/Maia.sol";
import "../src/hermes/tokens/HERMES.sol";
import "../src/hermes/bHermes.sol";
import "../src/hermes/tokens/bHermesGauges.sol";
import "../src/hermes/tokens/bHermesBoost.sol";
import {bHermesVotes as ERC20Votes} from "../src/hermes/tokens/bHermesVotes.sol";
import {IBaseV2Gauge} from "@gauges/interfaces/IBaseV2Gauge.sol";
import "../src/erc-20/interfaces/IERC20Gauges.sol";
import "../src/rewards/rewards/FlywheelGaugeRewards.sol";
import "../src/hermes/minters/BaseV2Minter.sol";
import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol";
import {UniswapV3Factory, UniswapV3Pool} from "@uniswap/v3-core/contracts/UniswapV3Factory.sol";

import {UniswapV3Factory, UniswapV3Pool} from "@uniswap/v3-core/contracts/UniswapV3Factory.sol";

import {IWETH9} from "@uniswap/v3-periphery/contracts/interfaces/external/IWETH9.sol";
import {ISwapRouter} from "@uniswap/v3-periphery/contracts/interfaces/ISwapRouter.sol";
import {
    NonfungiblePositionManager, IUniswapV3Pool
} from "@uniswap/v3-periphery/contracts/NonfungiblePositionManager.sol";
import {SwapRouter} from "@uniswap/v3-periphery/contracts/SwapRouter.sol";

import {TalosBaseStrategy} from "@talos/base/TalosBaseStrategy.sol";
import {TalosOptimizer} from "@talos/TalosOptimizer.sol";
import {PoolVariables, PoolActions} from "@talos/libraries/PoolActions.sol";
import {
    IUniswapV3Pool,
    UniswapV3Staker,
    IUniswapV3Staker,
    IncentiveTime,
    IncentiveId,
    bHermesBoost
} from "@v3-staker/UniswapV3Staker.sol";
import {TalosStrategyVanilla} from "@talos/TalosStrategyVanilla.sol";
import {UniswapV3GaugeFactory} from "@gauges/factories/UniswapV3GaugeFactory.sol";
import "@uniswap/v3-core/contracts/libraries/TickMath.sol";
import "../src/talos/libraries/PoolVariables.sol";
import "@uniswap/v3-periphery/contracts/libraries/PoolAddress.sol";
import "../src/rewards/booster/FlywheelBoosterGaugeWeight.sol";
import "../src/gauges/factories/BribesFactory.sol";
import "../src/gauges/factories/BaseV2GaugeManager.sol";
import "../src/weth9.sol";
import "../lib/v3-periphery/contracts/interfaces/INonfungiblePositionManager.sol";
import "../src/rewards/FlywheelCoreInstant.sol";





contract MaiaTest is Test {
    Maia _maia; 
    HERMES _hermes;
    bHermes _bhermes;

    bHermesGauges gaugeWeight;
    bHermesBoost  gaugeBoost;
    ERC20Votes    governance;

    MockERC20   _tokenA;
    MockERC20   _tokenB;
    UniswapV3Factory uniswapV3Factory;
    SwapRouter swapRouterContract;
    ISwapRouter swapRouter;
    NonfungiblePositionManager nonfungiblePositionManager;
    IUniswapV3Pool pool;
    UniswapV3Pool poolContract;
    TalosStrategyVanilla talosBaseStrategy;         /**???? */
    TalosOptimizer strategyOptimizer;            /** ??? */
    bHermesBoost _bHermesBoost;
    IUniswapV3Staker uniswapV3Staker;
    UniswapV3Staker uniswapV3StakerContract;
    FlywheelBoosterGaugeWeight flywheelGaugeWeightBooster;
    BribesFactory bribesFactory;
    UniswapV3GaugeFactory uniswapV3GaugeFactory;
    BaseV2GaugeManager baseV2GaugeManager;


    IWETH9 WETH9 = IWETH9(new weth9()); 

    address owner = address(0x901);   
    address depositor1 = address(0x101);
    address depositor2 = address(0x102);
    address depositor3 = address(0x103);
   
    address gauge1 = address(0x801);
    address gauge2 = address(0x802);
    address gauge3 = address(0x803);

    FlywheelGaugeRewards _flywheelGaugeRewards;
    BaseV2Minter _baseV2Minter;


    function setUp() public {
        _maia = new Maia(owner);
        _hermes = new HERMES(owner);
        _bhermes = new bHermes(_hermes, owner, 1 days, 2 hours);


        gaugeWeight = _bhermes.gaugeWeight();
        gaugeBoost = _bhermes.gaugeBoost();
        governance = _bhermes.governance();

  
        vm.startPrank(owner);
        
        _maia.mint(depositor1, 0);
        _maia.mint(depositor1, 1101e18);
        _maia.mint(depositor2, 1102e18);
        _maia.mint(depositor3, 1103e18);
        _maia.mint(address(this), 1_000_000e18);
        
        
        _hermes.mint(depositor1, 0);
        _hermes.mint(depositor1, 5_000_000e18);
        _hermes.mint(depositor2, 5_000_001e18);
        _hermes.mint(depositor3, 5_000_002e18);
        _hermes.mint(address(this), 5_000_003e18);

        // add gauges
        
        gaugeWeight.addGauge(gauge1);
        gaugeWeight.addGauge(gauge2);
        gaugeWeight.addGauge(gauge3);
        gaugeWeight.setMaxGauges(10);
        gaugeWeight.setMaxDelegates(10);
        
        vm.stopPrank();


        // create tokens
        _tokenA = new MockERC20("TokenA", "TKA", 18);
        _tokenB = new MockERC20("TokenB", "TKB", 18);

        _tokenA.mint(address(this), 25e18);
        _tokenB.mint(address(this), 25e18);
        vm.prank(owner);
        _hermes.mint(address(this), 25e18);
     

        uniswapV3Factory = new UniswapV3Factory();
        console2.log("factory address");
        console2.logAddress(address(uniswapV3Factory));
        swapRouterContract = new SwapRouter(address(uniswapV3Factory), address(WETH9));
        swapRouter = ISwapRouter(address(swapRouterContract));
     
        nonfungiblePositionManager = new NonfungiblePositionManager(
            address(uniswapV3Factory),   // uniswapV3Factory
            address(WETH9),         // weth9
            address(0)              // token descriptor
        );
        address poolAddress = uniswapV3Factory.createPool(address(WETH9), address(_tokenB), 3000);
        pool = IUniswapV3Pool(poolAddress);
        console2.log("pool address: ");
        console2.logAddress(address(pool));
        poolContract = UniswapV3Pool(poolAddress);
        poolContract.initialize(56022770974786139918731938227);  // inital price sqrtpirce x 96
        poolContract.increaseObservationCardinalityNext(100);
        // just a bunch of parameter values
        strategyOptimizer = new TalosOptimizer(100, 40, 16, 2000, type(uint256).max, address(this));

        
        _bHermesBoost = _bhermes.gaugeBoost();

        baseV2GaugeManager  = new BaseV2GaugeManager(_bhermes, address(this), address(this));

        
        
        
        /*
        uniswapV3StakerContract = new UniswapV3Staker(
            uniswapV3Factory,
            nonfungiblePositionManager,
            UniswapV3GaugeFactory(address(this)),
            _bHermesBoost,
            31536000,
            address(this),
            address(_hermes)
        );
        uniswapV3Staker = IUniswapV3Staker(address(uniswapV3StakerContract));
        */

        talosBaseStrategy = new TalosStrategyVanilla(
            pool,
            strategyOptimizer,
            nonfungiblePositionManager,
            address(this),                     // strategy manager
            address(this)
        );
       

        _baseV2Minter = new BaseV2Minter(address(_bhermes), owner, owner);  // reward token, dao, owner

        vm.prank(owner);
        _hermes.transferOwnership(address(_baseV2Minter)); // now only _baseV2Minter can mint HERMES

        _flywheelGaugeRewards = new FlywheelGaugeRewards(
            address(_hermes), // reward token
            address(this),     
            _bhermes.gaugeWeight(),  //  bHermesGauges
            _baseV2Minter           // BaseV2Minter
        );
        _baseV2Minter.initialize(_flywheelGaugeRewards);


        flywheelGaugeWeightBooster = new FlywheelBoosterGaugeWeight(_bhermes.gaugeWeight());

        bribesFactory = new BribesFactory(
            BaseV2GaugeManager(address(this)),
            flywheelGaugeWeightBooster,
            1 weeks,
            address(this)
        );

       


        uniswapV3GaugeFactory = new UniswapV3GaugeFactory(
            BaseV2GaugeManager(address(0)),       // zero address is the GaugeManager
            _bHermesBoost,
            uniswapV3Factory,
            nonfungiblePositionManager,
            _flywheelGaugeRewards,
            bribesFactory,
            address(this)
        );

        uniswapV3StakerContract = uniswapV3GaugeFactory.uniswapV3Staker();

        uniswapV3Staker = IUniswapV3Staker(address(uniswapV3StakerContract));

        baseV2GaugeManager =  new BaseV2GaugeManager(_bhermes, address(this), address(this));
       

        console2.log("circulating supply: %d", _baseV2Minter.circulatingSupply()); // underlying.totalsupply - vault(_bhermes).totalAssets(herm)

        console2.log("\n\nset up is done.\n\n");
        console2.log("****************************************************");      
    }


function testSetFlywheelRewards() public{
    FlywheelCoreInstant flywheelCore = new FlywheelCoreInstant(address(_hermes), IFlywheelRewards(address(0)), IFlywheelBooster(address(0)), address(this));
    FlywheelBribeRewards flywheelRewards = new FlywheelBribeRewards(flywheelCore, 1 weeks);
    _hermes.transfer(address(0), 1);
    vm.expectRevert();
    flywheelCore.setFlywheelRewards(address(flywheelRewards));
}
}

Tools Used

VSCode and foundry

Recommended Mitigation Steps

Add the condition to make sure the former address is non-zero address:

function setFlywheelRewards(address newFlywheelRewards) external onlyOwner {
        uint256 oldRewardBalance = rewardToken.balanceOf(address(flywheelRewards));
-        if (oldRewardBalance > 0) {
+        if (oldRewardBalance > 0 && flywheelRewards != address(0)) {

            rewardToken.safeTransferFrom(address(flywheelRewards), address(newFlywheelRewards), oldRewardBalance);
        }

        flywheelRewards = newFlywheelRewards;

        emit FlywheelRewardsUpdate(address(newFlywheelRewards));
    }

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/maia/tokens/ERC4626PartnerManager.sol#L325-L335

Vulnerability details

Impact

Detailed description of the impact of this finding. ERC4626PartnerManager.checkTransfer() does not check amount correctly as it applies bHermesRate to balanceOf[from] but not to amount.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

ERC4626PartnerManager.checkTransfer() is a modifier that will be called to ensure that the from account has sufficient funding to cover userClaimedWeight[from], userClaimedBoost[from], userClaimedGovernance[from], and userClaimedPartnerGovernance[from] before transfer occurs:

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/maia/tokens/ERC4626PartnerManager.sol#L325-L335

However, bHermesRate is applied to balanceOf[from] but not to amount. This is not right, since amount is not in the units of userClaimedWeight[from], userClaimedBoost[from], userClaimedGovernance[from], and userClaimedPartnerGovernance[from], but in the units of shares of ERC4626PartnerManager.

The correct way to check would be to ensure balanceOf[from]-amount * bHermesRate >= userClaimedWeight[from], userClaimedBoost[from], userClaimedGovernance[from], and userClaimedPartnerGovernance[from].

Tools Used

VSCode

Recommended Mitigation Steps

 modifier checkTransfer(address from, uint256 amount) virtual {
-        uint256 userBalance = balanceOf[from] * bHermesRate;
+        uint256 userBalance = (balanceOf[from] - amount) * bHermesRate;

-        if (
-            userBalance - userClaimedWeight[from] < amount || userBalance - userClaimedBoost[from] < amount
-                || userBalance - userClaimedGovernance[from] < amount
-                || userBalance - userClaimedPartnerGovernance[from] < amount
-        ) revert InsufficientUnderlying();

+        if (
+            userBalance < userClaimedWeight[from] || userBalance < userClaimedBoost[from] 
+                || userBalance < userClaimedGovernance[from]  || userBalance <  userClaimedPartnerGovernance[from] 
+        ) revert InsufficientUnderlying();


        _;
    }

Assessed type

Math

QA1. UlyssesPool.maxRedeem() needs to consider the protocol fees.

Impact: without considering the protocol fees, a user might redeem too much funds and then there is no sufficient left amount of funds for the protocol fees.

The correction is the following:

 function maxRedeem(address owner) public view override returns (uint256) {
-        return balanceOf[owner].min(asset.balanceOf(address(this)));
+        return balanceOf[owner].min(asset.balanceOf(address(this))- getProtocolFees());

    }

QA2. ERC20Boost.decrementGaugeBoost() fails to update getUserBoost[user]. As a result, freeGaugeBoost[user] and noAttached(user, amount) are not accurate and up-to-date. Then, a user might not be able to perform burn(), transfer() or transferFrom() for the boost tokens since these three functions are constrained by modifier noAttached(user, amount) but noAttached(user, amount) might fail due to such inaccuracy of freeGaugeBoost[user].

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/erc-20/ERC20Boost.sol#L175-L187

The following POC code illustrates the following scenario - see function testGaugeBoost():

1. depositor 1 deposits 1001 HERMES into _bhermes and claims 1001 boost tokens;

2. three gauges, gauge1, gauge2, and guage3 are added to the gaugeBoost;

3. Each gauge, gauge1, gauge2 and gauge2 attaches user depositor to itself, as a result, getUserBoost[depositor1] = 1001;

4. Gauge1, gauge2 and gauge3 calls gaugeBoost.decrementGaugeBoost() to decrement its guage boost by 1000 respectively. Supposedly, getUserBoost[depositor1] needs to be changed to 1 as a result, However, the gaugeBoost.decrementGaugeBoost() fails to do so, so getUserBoost[depositor1] = 1001 remains to be the same.

5. As a result, when depositor1 tries to transfer 50 boost tokens to depositor2, it fails although we expect it to be successful since we have 1000 free boost tokens for depositor1.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "forge-std/Test.sol";
import "../src/maia/tokens/Maia.sol";
import "../src/hermes/tokens/HERMES.sol";
import "../src/hermes/bHermes.sol";
import "../src/hermes/tokens/bHermesGauges.sol";
import "../src/hermes/tokens/bHermesBoost.sol";
import {bHermesVotes as ERC20Votes} from "../src/hermes/tokens/bHermesVotes.sol";
import {IBaseV2Gauge} from "@gauges/interfaces/IBaseV2Gauge.sol";
import "../src/erc-20/interfaces/IERC20Gauges.sol";
import "../src/rewards/rewards/FlywheelGaugeRewards.sol";
import "../src/hermes/minters/BaseV2Minter.sol";
import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol";
import {UniswapV3Factory, UniswapV3Pool} from "@uniswap/v3-core/contracts/UniswapV3Factory.sol";

import {UniswapV3Factory, UniswapV3Pool} from "@uniswap/v3-core/contracts/UniswapV3Factory.sol";

import {IWETH9} from "@uniswap/v3-periphery/contracts/interfaces/external/IWETH9.sol";
import {ISwapRouter} from "@uniswap/v3-periphery/contracts/interfaces/ISwapRouter.sol";
import {
    NonfungiblePositionManager, IUniswapV3Pool
} from "@uniswap/v3-periphery/contracts/NonfungiblePositionManager.sol";
import {SwapRouter} from "@uniswap/v3-periphery/contracts/SwapRouter.sol";

import {TalosBaseStrategy} from "@talos/base/TalosBaseStrategy.sol";
import {TalosOptimizer} from "@talos/TalosOptimizer.sol";
import {PoolVariables, PoolActions} from "@talos/libraries/PoolActions.sol";
import {
    IUniswapV3Pool,
    UniswapV3Staker,
    IUniswapV3Staker,
    IncentiveTime,
    IncentiveId,
    bHermesBoost
} from "@v3-staker/UniswapV3Staker.sol";
import {TalosStrategyVanilla} from "@talos/TalosStrategyVanilla.sol";
import {UniswapV3GaugeFactory} from "@gauges/factories/UniswapV3GaugeFactory.sol";
import "@uniswap/v3-core/contracts/libraries/TickMath.sol";
import "../src/talos/libraries/PoolVariables.sol";
import "@uniswap/v3-periphery/contracts/libraries/PoolAddress.sol";
import "../src/rewards/booster/FlywheelBoosterGaugeWeight.sol";
import "../src/gauges/factories/BribesFactory.sol";
import "../src/gauges/factories/BaseV2GaugeManager.sol";
import "../src/weth9.sol";
import "../lib/v3-periphery/contracts/interfaces/INonfungiblePositionManager.sol";
import "../src/rewards/FlywheelCoreInstant.sol";





contract MaiaTest is Test {
    Maia _maia; 
    HERMES _hermes;
    bHermes _bhermes;

    bHermesGauges gaugeWeight;
    bHermesBoost  gaugeBoost;
    ERC20Votes    governance;

    MockERC20   _tokenA;
    MockERC20   _tokenB;
    UniswapV3Factory uniswapV3Factory;
    SwapRouter swapRouterContract;
    ISwapRouter swapRouter;
    NonfungiblePositionManager nonfungiblePositionManager;
    IUniswapV3Pool pool;
    UniswapV3Pool poolContract;
    TalosStrategyVanilla talosBaseStrategy;         /**???? */
    TalosOptimizer strategyOptimizer;            /** ??? */
    bHermesBoost _bHermesBoost;
    IUniswapV3Staker uniswapV3Staker;
    UniswapV3Staker uniswapV3StakerContract;
    FlywheelBoosterGaugeWeight flywheelGaugeWeightBooster;
    BribesFactory bribesFactory;
    UniswapV3GaugeFactory uniswapV3GaugeFactory;
    BaseV2GaugeManager baseV2GaugeManager;


    IWETH9 WETH9 = IWETH9(new weth9()); 

    address owner = address(0x901);   
    address depositor1 = address(0x101);
    address depositor2 = address(0x102);
    address depositor3 = address(0x103);
   
    address gauge1 = address(0x801);
    address gauge2 = address(0x802);
    address gauge3 = address(0x803);

    FlywheelGaugeRewards _flywheelGaugeRewards;
    BaseV2Minter _baseV2Minter;


    function setUp() public {
        _maia = new Maia(owner);
        _hermes = new HERMES(owner);
        _bhermes = new bHermes(_hermes, owner, 1 days, 2 hours);


        gaugeWeight = _bhermes.gaugeWeight();
        gaugeBoost = _bhermes.gaugeBoost();
        governance = _bhermes.governance();

  
        vm.startPrank(owner);
        
        _maia.mint(depositor1, 0);
        _maia.mint(depositor1, 1101e18);
        _maia.mint(depositor2, 1102e18);
        _maia.mint(depositor3, 1103e18);
        _maia.mint(address(this), 1_000_000e18);
        
        
        _hermes.mint(depositor1, 0);
        _hermes.mint(depositor1, 5_000_000e18);
        _hermes.mint(depositor2, 5_000_001e18);
        _hermes.mint(depositor3, 5_000_002e18);
        _hermes.mint(address(this), 5_000_003e18);

       
        // add gauges
        
        gaugeWeight.addGauge(gauge1);
        gaugeWeight.addGauge(gauge2);
        gaugeWeight.addGauge(gauge3);
        gaugeWeight.setMaxGauges(10);
        gaugeWeight.setMaxDelegates(10);
        
        vm.stopPrank();


        // create tokens
        _tokenA = new MockERC20("TokenA", "TKA", 18);
        _tokenB = new MockERC20("TokenB", "TKB", 18);

        _tokenA.mint(address(this), 25e18);
        _tokenB.mint(address(this), 25e18);
        vm.prank(owner);
        _hermes.mint(address(this), 25e18);
     

        uniswapV3Factory = new UniswapV3Factory();
        console2.log("factory address");
        console2.logAddress(address(uniswapV3Factory));
        swapRouterContract = new SwapRouter(address(uniswapV3Factory), address(WETH9));
        swapRouter = ISwapRouter(address(swapRouterContract));
     
        nonfungiblePositionManager = new NonfungiblePositionManager(
            address(uniswapV3Factory),   // uniswapV3Factory
            address(WETH9),         // weth9
            address(0)              // token descriptor
        );
        address poolAddress = uniswapV3Factory.createPool(address(WETH9), address(_tokenB), 3000);
        pool = IUniswapV3Pool(poolAddress);
        console2.log("pool address: ");
        console2.logAddress(address(pool));
        poolContract = UniswapV3Pool(poolAddress);
        poolContract.initialize(56022770974786139918731938227);  // inital price sqrtpirce x 96
        poolContract.increaseObservationCardinalityNext(100);
        // just a bunch of parameter values
        strategyOptimizer = new TalosOptimizer(100, 40, 16, 2000, type(uint256).max, address(this));

        
        _bHermesBoost = _bhermes.gaugeBoost();

        baseV2GaugeManager  = new BaseV2GaugeManager(_bhermes, address(this), address(this));

        
        
        
        /*
        uniswapV3StakerContract = new UniswapV3Staker(
            uniswapV3Factory,
            nonfungiblePositionManager,
            UniswapV3GaugeFactory(address(this)),
            _bHermesBoost,
            31536000,
            address(this),
            address(_hermes)
        );
        uniswapV3Staker = IUniswapV3Staker(address(uniswapV3StakerContract));
        */

        talosBaseStrategy = new TalosStrategyVanilla(
            pool,
            strategyOptimizer,
            nonfungiblePositionManager,
            address(this),                     // strategy manager
            address(this)
        );
       

        _baseV2Minter = new BaseV2Minter(address(_bhermes), owner, owner);  // reward token, dao, owner

        vm.prank(owner);
        _hermes.transferOwnership(address(_baseV2Minter)); // now only _baseV2Minter can mint HERMES

        _flywheelGaugeRewards = new FlywheelGaugeRewards(
            address(_hermes), // reward token
            address(this),     
            _bhermes.gaugeWeight(),  //  bHermesGauges
            _baseV2Minter           // BaseV2Minter
        );
        _baseV2Minter.initialize(_flywheelGaugeRewards);


        flywheelGaugeWeightBooster = new FlywheelBoosterGaugeWeight(_bhermes.gaugeWeight());

        bribesFactory = new BribesFactory(
            BaseV2GaugeManager(address(this)),
            flywheelGaugeWeightBooster,
            1 weeks,
            address(this)
        );

       


        uniswapV3GaugeFactory = new UniswapV3GaugeFactory(
            BaseV2GaugeManager(address(0)),       // zero address is the GaugeManager
            _bHermesBoost,
            uniswapV3Factory,
            nonfungiblePositionManager,
            _flywheelGaugeRewards,
            bribesFactory,
            address(this)
        );

        uniswapV3StakerContract = uniswapV3GaugeFactory.uniswapV3Staker();

        uniswapV3Staker = IUniswapV3Staker(address(uniswapV3StakerContract));

        baseV2GaugeManager =  new BaseV2GaugeManager(_bhermes, address(this), address(this));
       

        console2.log("circulating supply: %d", _baseV2Minter.circulatingSupply()); // underlying.totalsupply - vault(_bhermes).totalAssets(herm)

        console2.log("\n\nset up is done.\n\n");
        console2.log("****************************************************");      
    }


function testGaugeBoost() public{
    uint deposit1Amount = 1001;

     vm.startPrank(depositor1);
    _hermes.approve(address(_bhermes), deposit1Amount);
    _bhermes.deposit(deposit1Amount, address(depositor1));       // depositor hermest into vault
    console2.log("depositor1 claim weight: %d", deposit1Amount);
    _bhermes.claimBoost(deposit1Amount);             // claim weight based on bherms balance
    vm.stopPrank();
    console2.log("boost balance of depositor1: %d", gaugeBoost.balanceOf(depositor1));

    vm.startPrank(owner);
    gaugeBoost.addGauge(gauge1);
    gaugeBoost.addGauge(gauge2);
    gaugeBoost.addGauge(gauge3);
    vm.stopPrank();
 
    vm.prank(address(gauge1));
    gaugeBoost.attach(address(depositor1));
    console2.log("getUserBoost[depositor1]: %d", gaugeBoost.getUserBoost(depositor1));

    vm.prank(address(gauge2));
    gaugeBoost.attach(address(depositor1));
    console2.log("getUserBoost[depositor1]: %d", gaugeBoost.getUserBoost(depositor1));
    
    vm.prank(address(gauge3));
    gaugeBoost.attach(address(depositor1));
    console2.log("getUserBoost[depositor1]: %d", gaugeBoost.getUserBoost(depositor1));

    vm.startPrank(address(depositor1));
    gaugeBoost.decrementGaugeBoost(address(gauge1), 1000);
    gaugeBoost.decrementGaugeBoost(address(gauge2), 1000);
    gaugeBoost.decrementGaugeBoost(address(gauge3), 1000);
    vm.stopPrank();
    console2.log("getUserBoost[depositor1]: %d", gaugeBoost.getUserBoost(depositor1));
    
    vm.prank(address(depositor1));
    vm.expectRevert();
    gaugeBoost.transfer(depositor2, 50);
 
}
}

QA3. BaseV2Minter.getRewards() does not calculate totalQueuedForCycle correctly.

This is because the BaseV2Minter contract might not have sufficient coverage for the weekly amount. In this case, totalQueuedForCycle should be the balance of underlying in the BaseV2Minter contract. Correction:

function getRewards() external returns (uint256 totalQueuedForCycle) {
        if (address(flywheelGaugeRewards) != msg.sender) revert NotFlywheelGaugeRewards();
        totalQueuedForCycle = weekly;
+       uint256 bal = underlying.balanceOf(address(this));
+       if(bal < totalQueuedForCycle) totalQueuedForCycle = bal;
-        weekly = 0;
+       weekly -= totalQueuedForCycle;
        underlying.safeTransfer(msg.sender, totalQueuedForCycle);
    }

QA3. Much confusion is introduced due to renaming in import:

1. FlywheelCore is renamed as core:

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/rewards/FlywheelCoreInstant.sol#L10C1-L10C62

2. FlyWheelStrategy is renamed as FlyWheelCore sometimes.

To avoid this confusion, we eliminate all renamings.

QA4: When currentTick is too high or too low, TalosManager.getRebalance() will underflow instead of returning true. As a result, performUpkeep() might fail even in the case when it is supposed to call strategy.reBalance() successfully;

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/talos/TalosManager.sol#L66-L72

Correction: check to make sure underflow will not occur:

function getRebalance(ITalosBaseStrategy position) private view returns (bool) {
        //Calculate base ticks.
        (, int24 currentTick,,,,,) = position.pool().slot0();

-        return currentTick - position.tickLower() >= ticksFromLowerRebalance
-            || position.tickUpper() - currentTick >= ticksFromUpperRebalance;

+ if(currentTick > position.tickLower() && currentTick - position.tickLower() >= ticksFromLowerRebalance) return true;

+ if(position.tickUpper() > currentTick && position.tickUpper() - currentTick >= ticksFromUpperRebalance) return true; 

+ return false;
    }

QA5. When currentTick is too high or too low, TalosManager.getRerange() will underflow instead of returning true. As a result, performUpkeep() might fail even in the case when it is supposed to call strategy.re Range() successfully;

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/talos/TalosManager.sol#L78-L84

Correction: check to make sure underflow will not occur: function getRerange(ITalosBaseStrategy position) private view returns (bool) { //Calculate base ticks. (, int24 currentTick,,,,,) = position.pool().slot0();

•    return currentTick - position.tickLower() >= ticksFromLowerRerange
  

•        || position.tickUpper() - currentTick >= ticksFromUpperRerange;
  

•    if(currentTick > position.tickLower() && currentTick - position.tickLower() >= ticksFromLowerRerange) return true;
  

•    if(position.tickUpper() > currentTick && position.tickUpper() - currentTick >= ticksFromUpperRerange) return true;
  

• return false; }

QA6. BoostAggregator.unstakeAndWithdraw() fails to update tokenIdRewards[tokenId]. After unstaking and withdrawing a token, tokenIdRewards[tokenId] is supposed to be synced to uniswapV3Staker.tokenIdRewards(tokenId), but the function BoostAggregator.unstakeAndWithdraw() fails to do so.

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/talos/boost-aggregator/BoostAggregator.sol#L109-L136

The correction would be to sync tokenIdRewards[tokenId] to uniswapV3Staker.tokenIdRewards(tokenId) in the function

QA7: BoostAggregator.unstakeAndWithdraw() fails to check the return value of uniswapV3Staker.claimReward() (at L127 and L130). As a result, the actual claimed reward might be less than pendingRewards. The accouting for pending rewards is thus not accrurate and a user might receive less rewards than he deserves. There is no other function to claim the remaining rewards.

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/talos/boost-aggregator/BoostAggregator.sol#L109-L136

The funtion calls uniswapV3Staker.claimReward() to claim rewards, but when the uniswapV3Staker contract has a reward balance less than pendingRewards for BoostAggregator , the actual claimed rewards will be less than pendingRewards, see the implementatino of uniswapV3Staker.claimReward():

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/uni-v3-staker/UniswapV3Staker.sol#L262-L274

The correction would be to check the return value uniswapV3Staker.claimReward() so that we know how much actual reward is claimed and then update tokenIdRewards[tokenId]. Another function to claim the remaining rewards need to be introduced as well.

QA8: UniswapV3Staker.updateGauges() fails to delete gaugePool[gauges[uniswapV3Pool]]. As a result, both the old gauge and the new gauge will point to the same uniswapV3Pool.

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/uni-v3-staker/UniswapV3Staker.sol#L526-L540

Consider pool1 and suppose gauges[pool1] = gauge1, and gauge2 = address(uniswapV3GaugeFactory.strategyGauges(address(pool1))). In other words, we need to update the gauge from gauge1 to gauge2. However, since UniswapV3Staker.updateGauges() fails to delete gaugePool[gauge1], we will have gaugePool[gauge1] = pool1, and gaugePool[guauge2] = pool1.

The correction would be to delete gaugePool[gauge1] as follows:

function updateGauges(IUniswapV3Pool uniswapV3Pool) external {
        address uniswapV3Gauge = address(uniswapV3GaugeFactory.strategyGauges(address(uniswapV3Pool)));

        if (uniswapV3Gauge == address(0)) revert InvalidGauge();
 
        if (address(gauges[uniswapV3Pool]) != uniswapV3Gauge) {
            emit GaugeUpdated(uniswapV3Pool, uniswapV3Gauge);
+           delete gaugePool[address(gauges[uniswapV3Pool])];         // delete gaugePool[gauge1]
            gauges[uniswapV3Pool] = UniswapV3Gauge(uniswapV3Gauge);
            gaugePool[uniswapV3Gauge] = uniswapV3Pool;
        }

        updateBribeDepot(uniswapV3Pool);
        updatePoolMinimumWidth(uniswapV3Pool);
    }

QA9. BoostAggregator.decrementGaugesBoostIndexed() fails to call hermesGaugeBoost.updateUserBoost(address(this)). As a result, getUserBoost[address(this)] is not updated and thus not accurate after the call of BoostAggregator.decrementGaugesBoostIndexed().

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/talos/boost-aggregator/BoostAggregator.sol#L180-L182

Correction: we need to call hermesGaugeBoost.updateUserBoost(address(this)) in BoostAggregator.decrementGaugesBoostIndexed():

  function decrementGaugesBoostIndexed(uint256 boost, uint256 offset, uint256 num) external onlyOwner {
        hermesGaugeBoost.decrementGaugesBoostIndexed(boost, offset, num);
+       hermesGaugeBoost.updateUserBoost(address(this));
    }