search

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

5 stars 0 forks source link

Contract Owner can change the fee and cause an absolute loss for option seller #426

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#L240

Vulnerability details

Impact

Admin can cause significant loss for option sellers compared to premium

Proof of Concept

it’s possible for seller to suffer a net loss even if the option doesn’t got exercised because of the admin power of setting fee. There is also no check on whether strike * fee is smaller or greater than the premium the seller get, which means sellers can accidentally put an order that is absolutely not worth it.

e.g.: I sell a 100ETH put on an NFT for 3 days with premium of 3 eth, After expiry and before taking out the 100 ETH, owner change the fee rate to maximum (3%), i got charge another 3% which is my premium, ended in an absolute net loss that I didn’t want to agree on.

Recommended Mitigation Steps

it’s better to embed a “fee” parameter in the order structure, and when filling order, make sure order.fee == fee, so no one can avoid paying fee after the protocol turn the switch on, and then check premium > strike * fee to make sure sellers are not doing something stupid.

while withdrawing, use the fee in the order struct to make sure this rate is agreed by both parties when filling the order. This should remove all concerns regarding fees.

outdoteth commented 2 years ago

Duplicate: Admin can change fee at any time for existing orders: https://github.com/code-423n4/2022-06-putty-findings/issues/422