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

5 stars 0 forks source link

The strike cover amount can be locked for very long time in the putty contract #352

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

This is an edge case, but calls for attention to fine tune a max parameter.

If a maker creates and signs a Long Put Option for a very long duration say (10,000-1) days, and the taker by mistake fills the order, then strike amount is sent to the contract, and if the maker does not exercise the option, the amount is locked for very long time. The amount of premium the taker received may not be economically worth it. The max duration check is for 10,000 days which is around 27+ years, which is very very long wrt to options.

Proof of Concept

Contract : PuttyV2.sol Function : fillOrder() Line# 287

   require(order.duration < 10_000 days, "Duration too long");

Recommended Mitigation Steps

Have a more realistic max value check for the order.duration say 2 years or 3 years.

outdoteth commented 2 years ago

What constitutes a "long time" is subjective.

Given the immutable nature of smart contracts we wanted to choose a limit that provided as much flexibility as possible while still being within realistic bounds.

2-3 years, while reasonable, is not quite flexible enough. It is entirely possible that, in an edge case, someone may want an option with a longer duration than this. ~27 years provides a lot more flexibility while still being within a "reasonable" bound (e.g. vs max uint256).

outdoteth commented 2 years ago

Is this still a valid finding after my comment above?^^

HickupHH3 commented 2 years ago

Similar to #88 where the max period of 10_000 days is considered to be too long by the wardens. If the sponsor deems it reasonable after considering the risks that the wardens have pointed out, then there isn't much left to dispute.

Downgrading to non-crit QA because it's not entirely invalid, more in tune with "sponsor acknowledged".

HickupHH3 commented 2 years ago

part of warden's QA: #428

ksk2345 commented 2 years ago

In traditional finance, i can see options expiring in monthly, quarterly and 1 year cycles. I am not aware of any options going for longer than 2 to 3 years also. More longer would be like perpetual. The bug was filed to stop any griefing attack or if by mistake some wrong values are given. Having a duration like 10,000 or 27 years, can only make sense if you are going to market it as a feature differentiator wrt to some other competing defi protocols in this space.

outdoteth commented 2 years ago

@ksk2345 there are LEAPs contracts and ESO contracts which can last for very long durations. Also consider hedging 10Y bonds etc. Then it makes sense to have option contracts that last in the 27 year range.