Lines of code

https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L599

Vulnerability details

Impact

HIGH - assets can be compromised By creating an order which can be filled but not possible to exercise, the maker of this order is basically getting free premium. Even if the taker wants to exercise the long put option, it will revert. After the option is expired, the maker can withdraw strike. Malicious users can make attractive deals to lure people to take their orders then wait until it expires and takes back all strikes. This will also hurt the reputation of the platform.

It is similar case with Create an short call order with non empty floor makes the option impossible to exercise and withdraw but the root cause is different, also the maker cannot withdraw after expiration.

Proof of Concept

The proof of concept shows a scenario, where babe makes a short put order with 0 tokenAmount. Bob filled the order and he has an NFT of long put. When he calls the exercise, the putty contract tries to transfer ERC20s from bob to itself. However the _transferERC20sIn function reverts, because it requires the tokenAmount to be greater than zero.

// When bob calls exercise
// PuttyV2.sol: _transferERC20sIn function called by exercise
//  require(tokenAmount > 0, "ERC20: Amount too small");

    function _transferERC20sIn(ERC20Asset[] memory assets, address from) internal {
        for (uint256 i = 0; i < assets.length; i++) {
            address token = assets[i].token;
            uint256 tokenAmount = assets[i].tokenAmount;

            require(token.code.length > 0, "ERC20: Token is not contract");
            require(tokenAmount > 0, "ERC20: Amount too small");

            ERC20(token).safeTransferFrom(from, address(this), tokenAmount);
        }
    }

Since bob can never exercise his option, babe does not need to worry about the price movement. She can also make attractive deals without worrying it will realize. Basically she is taking free premium. After the option is expired, the maker of the order, babe, can now withdraw the strike (minus fees, based on the current implementation, but the fee should not be paid when not exercised - see the issue The fee is not paid as intended for put orders).

Note:

In the reference case, the tokenAmount is set to 1, instead of 0 and the taker of the order can exercise as intended.

Tools Used

foundry

Recommended Mitigation Steps

The easy mitigation would be getting rid of the requirement: require(tokenAmount > 0, "ERC20: Amount too small"); Alternatively, ensure the orders to have non zero tokenAmount.

code-423n4 / 2022-06-putty-findings