Abracadabra Mimswap - Limbooo's results

Lines of code

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

Vulnerability details

In the Router.sol file of the mimswap, there's a method to create a pool for native tokens by wrapping them to their "wrapped" counterpart before sending them to the newly created pool.

src/mimswap/periphery/Router.sol:
  73:     function createPoolETH(
  74:         address token,
  75:         bool useTokenAsQuote,
  76:         uint256 lpFeeRate,
  77:         uint256 i,
  78:         uint256 k,
  79:         address to,
  80:         uint256 tokenInAmount
  81:     ) external payable returns (address clone, uint256 shares) {
  82:         if (useTokenAsQuote) {
  83:             _validateDecimals(18, IERC20Metadata(token).decimals());
  84:         } else {
  85:             _validateDecimals(IERC20Metadata(token).decimals(), 18);
  86:         }
  87: 
  88:         clone = IFactory(factory).create(useTokenAsQuote ? address(weth) : token, useTokenAsQuote ? token : address(weth), lpFeeRate, i, k);
  89: 
  90:         weth.deposit{value: msg.value}();
  91:         token.safeTransferFrom(msg.sender, clone, tokenInAmount);
  92:         address(weth).safeTransferFrom(address(this), clone, msg.value);
  93:         (shares, , ) = IMagicLP(clone).buyShares(to);
  94:     }

However, the transfer done using address(weth).safeTransferFrom (see line 92). This works fine on most chains (Ethereum, Optimism, Polygon, BSC) which uses the standard WETH9 contract that handles the case  when src == msg.sender:

WETH9.sol
        if (src != msg.sender && allowance[src][msg.sender] != uint(- 1)) {
            require(allowance[src][msg.sender] >= wad);
            allowance[src][msg.sender] -= wad;
        }

The problem is that the WETH implementation on Blast uses a different contract, and does not have this src == msg.sender handling.

Also, the issue is presented in Wrapped Arbitrum and  Wrapped Fantom.

Impact

The failure to approve the Router contract to spend WETH tokens will prevent the protocol from creating native tokens pools on multiple chains like Blast.

Proof of Concept

Run test using this command

forge test --match-test testPoC_TransferFromRevert --fork-url https://rpc.blast.io

pragma solidity ^0.8.0;

import "forge-std/Test.sol";
import "forge-std/console.sol";
import {IERC20} from "forge-std/interfaces/IERC20.sol";

contract PairTest is Test {
    address alice = address(0xf683Ce59521AA464066783d78e40CD9412f33D21);
    address bob = address(0x2);
    // WETH address on Blast network
    IERC20 public constant WETH = IERC20(0x4300000000000000000000000000000000000004);
    error InsufficientAllowance();

    function testPoC_TransferFromRevert() public {
        // stdstore write for packed slot is complex so we use a real address that has tokens in blaset main net weth
        // if this fails we need to update alice address to an address that has more than 1 ether balance in weth blast main net
        assert(WETH.balanceOf(alice) > 1 ether);

        vm.startPrank(alice);
        vm.expectRevert(InsufficientAllowance.selector);
        WETH.transferFrom(alice, bob, 1 ether);
        vm.stopPrank();
    }
}

Test output

2024-03-abracadabra-money main* 5s
❯ forge test --match-test testPoC_TransferFromRevert --fork-url https://rpc.blast.io -vvvv
[⠊] Compiling...
[⠑] Compiling 1 files with 0.8.20
[⠘] Solc 0.8.20 finished in 598.78ms
Compiler run successful!

Ran 1 test for test/weth.t.sol:PairTest
[PASS] testPoC_TransferFromRevert() (gas: 25796)
Traces:
  [25796] PairTest::testPoC_TransferFromRevert()
    ├─ [9930] 0x4300000000000000000000000000000000000004::balanceOf(0xf683Ce59521AA464066783d78e40CD9412f33D21) [staticcall]
    │   ├─ [4910] 0x83acB050AA232F97810F32aFACDE003303465ca5::balanceOf(0xf683Ce59521AA464066783d78e40CD9412f33D21) [delegatecall]
    │   │   └─ ← 3234865746423262842620 [3.234e21]
    │   └─ ← 3234865746423262842620 [3.234e21]
    ├─ [0] VM::startPrank(0xf683Ce59521AA464066783d78e40CD9412f33D21)
    │   └─ ← ()
    ├─ [0] VM::expectRevert(InsufficientAllowance())
    │   └─ ← ()
    ├─ [3470] 0x4300000000000000000000000000000000000004::transferFrom(0xf683Ce59521AA464066783d78e40CD9412f33D21, 0x0000000000000000000000000000000000000002, 1000000000000000000 [1e18])
    │   ├─ [2944] 0x83acB050AA232F97810F32aFACDE003303465ca5::transferFrom(0xf683Ce59521AA464066783d78e40CD9412f33D21, 0x0000000000000000000000000000000000000002, 1000000000000000000 [1e18]) [delegatecall]
    │   │   └─ ← InsufficientAllowance()
    │   └─ ← InsufficientAllowance()
    ├─ [0] VM::stopPrank()
    │   └─ ← ()
    └─ ← ()

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.84s (1.28s CPU time)

Ran 1 test suite in 3.81s (2.84s CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Similar Findings

• https://github.com/code-423n4/2023-05-chainlink-findings/issues/593

Tools Used

• Manual review
• Network scan
• Foundry

Recommended Mitigation Steps

To address this issue, it's recommended to modify the Router.sol file as follows:

src/mimswap/periphery/Router.sol:
-92:         address(weth).safeTransferFrom(address(this), clone, msg.value);
+92:         address(weth).safeTransfer(clone, msg.value);

Assessed type

Error