An order maker might steal premium from the sender by setting order.strike = 0.

[code423n4]

Lines of code

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

Vulnerability details

Impact

An order maker might steal premium from the sender by setting order.strike = 0.
It's known that some ERC20 tokens don't allow 0 transfer and it will revert when the order sender tries to exercise his long position.
So the order maker will receive back his tokens after the order is expired.

Proof of Concept

As we can see here, some tokens don't allow 0 transfer. Let's call the token TOK and assume 1 TOK = 1 USDC.
And I can't find a restriction on that order.strike must be positive and I think it would be possible to create an order that has a positive premium and zero strikes.

1. Alice creates a short call order with the below settings.
   premium = 5000 TOK, strike = 0 TOK, ERC20 token = 10000 USDC
2. Because this order is profitable for senders, Bob fills this order by sending 5000 TOK premium.
3. After that, Bob tries to exercise the order but will revert because of this one.
4. After the order is expired, Alice withdraws her 10000 USDC.

Tools Used

Solidity Visual Developer of VSCode

Recommended Mitigation Steps

We should check order.strike != 0 before transfer.
Modification for L436.

if(order.strike != 0) {
    ERC20(order.baseAsset).safeTransferFrom(msg.sender, address(this), order.strike);
}

In the fillOrder() function, there are similar transfers here and here, but there will be no loss of funds because the order won't be filled anyway.

[ghost]

duplicate of #418

[dmitriia]

To add a bit here, zero strike options are pretty common and valid derivative type.

[outdoteth]

Duplicate: Cannot exercise call contract if strike is 0 and baseAsset reverts on 0 transfers: #418