withdraw() could be blocked if feeAmount would be 0 and baseAsset contract doesn’t allow for zero amount transfers

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L497-L501

Vulnerability details

Impact

In withdraw() function if order is exercised call or not exercised long and fee > 0, contract takes some feeAmount to owner address, charged in baseAsset. But in case when feeAmount would equal zero (due to low strike price and low fee) and baseAsset contract doesn’t allow for zero amount transfers, withdraw function would be blocked.

Proof of Concept

If fee is 1 and order.strike is 999 wei, feeAmount would be counted as 0 and transfer to owner() address will always revert. Setting fee to 0 value can be impossible or quite hard if owner() address would be controlled by Dao.

            ...
            uint256 feeAmount = 0;
            if (fee > 0) {
                feeAmount = (order.strike * fee) / 1000; // @audit note magic number
                ERC20(order.baseAsset).safeTransfer(owner(), feeAmount); // @audit low malicious owner can broke
            }
            ...

Recommended Mitigation Steps

Add check for feeAmount > 0:

            uint256 feeAmount = 0;
            if (fee > 0) {
                feeAmount = (order.strike * fee) / 1000;
                if (feeAmount > 0) {
                    ERC20(order.baseAsset).safeTransfer(owner(), feeAmount);
                }
            }

[outdoteth]

Duplicate: withdraw() can be DOS’d for baseAsset ERC20s that prevent 0 transfers if the calculated feeAmount is 0 due to rounding: https://github.com/code-423n4/2022-06-putty-findings/issues/283