Malicious Order Maker Can Set The Position To Expired Immediately After Being Filled

[code423n4]

Lines of code

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

Vulnerability details

Proof-of-Concept

The PuttyV2.fillOrder function will validate that the order duration is less than 10,000 days. However, it does not check that order duration is equal to zero. Thus, it is possible for a malicious order maker to create an order with order.duration equal to zero. This will cause the order to expired immediately after being filled by a taker.

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

function fillOrder(
    Order memory order,
    bytes calldata signature,
    uint256[] memory floorAssetTokenIds
) public payable returns (uint256 positionId) {
   ..SNIP..
    // check duration is valid
    require(order.duration < 10_000 days, "Duration too long");
    ..SNIP..
    // save the long position expiration
    positionExpirations[order.isLong ? uint256(orderHash) : positionId] = block.timestamp + order.duration;
    ..SNIP..
}

An attacker (Alice) could perform the following:

1. Alice (order maker) create an "Short Call" order with order.duration set to zero.
2. Bob (order taker) make a mistake by filling Alice's order, and send the premium to Alice. Upon filling Alice's order, Bob's long call option expires immediately, and there is no chance for him to exercise his option.
3. Alice gains the premium in a risk-free environment, and get to keep her assets.

Impact

The order taker (victim) would lost their funds as they paid for an option that cannot be exercised because the option expired immediately after being bought.

Recommended Mitigation Steps

It is recommended to implement additional validation to ensure that the order.duration is not zero or set to short period of time (e.g. 5 minute).

An option that expires immediately or expires within a short period of time (e.g. 10 seconds or 5 minutes) does not have much value, and thus it should not be allowed within Putty. Order makers who create such orders are likely to be malicious user who exploits this flexibility to 'trick' or 'fish' the users into filling their order to obtain the premium, thus causing harm to the community.

[GalloDaSballo]

Why would the counter-party accept a 10 seconds or 5 minutes contract unless they wanted to buy it?

[outdoteth]

Duplicate: Orders with low durations can be easily DOS’d and prevent possibility of exercise: https://github.com/code-423n4/2022-06-putty-findings/issues/265