QA Report - Githubissues

User shouldn't be able to fill an order with a strike = 0 or premium = 0, as it creates inconsistencies in the code when handling native ETH. For example in fillOrder() https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L322-L340

        // transfer premium to whoever is short from whomever is long
        if (order.isLong) {
            ERC20(order.baseAsset).safeTransferFrom(order.maker, msg.sender, order.premium);
        } else {
            // handle the case where the user uses native ETH instead of WETH to pay the premium
            if (weth == order.baseAsset && msg.value > 0) {
                // check enough ETH was sent to cover the premium
                require(msg.value == order.premium, "Incorrect ETH amount sent");

                // convert ETH to WETH and send premium to maker
                // converting to WETH instead of forwarding native ETH to the maker has two benefits;
                // 1) active market makers will mostly be using WETH not native ETH
                // 2) attack surface for re-entrancy is reduced
                IWETH(weth).deposit{value: msg.value}();
                IWETH(weth).transfer(order.maker, msg.value);
            } else {
                ERC20(order.baseAsset).safeTransferFrom(msg.sender, order.maker, order.premium);
            }
        }

A user is allowed to fill the order with premium 0 and if the order.baseAsset is weth, because the msg.value has to be grater than 0, it's going to skip that and execute the ERC20 transfer instead.

I found 3 instances of that issue:

Fix:

In order to fix that just add a check in the fillOrder() to restrict strike and premiums with 0 value. require(order.strike > 0 && order.premium > 0)

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

QA Report #184

Fix: