search

code-423n4 / 2022-05-cally-findings

2 stars 0 forks source link

function `exercise` might fail sporadically due to UI & transaction delays. #313

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L272

Vulnerability details

Impact

The function exercise is checking for an exact match of msg.value for it to be successful.

 // check correct ETH amount was sent to pay the strike
 require(msg.value == vault.currentStrike, "Incorrect ETH sent for strike");

There can be a time delay between when user submit, and the transaction to complete. By this time, if there is a new block, there will be new currentStrike price. The condition will fail, and the user will be denied the exercise.

This can happen quite often based on the user interface.

Proof of Concept

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L272

Tools Used

Manual review

Recommended Mitigation Steps

Receive a msg.value greater than or equal to the currentStrike

// check correct ETH amount was sent to pay the strike require(msg.value >= vault.currentStrike, "Incorrect ETH sent for strike");

Send back the difference amout to user.

outdoteth commented 2 years ago

Once an option is bought the currentStrike is locked in so it cannot be variable as suggested in the issue.

itsmetechjay commented 2 years ago

Withdrawn by warden. Per warden: "I made a recent high submission for Cally contest titled 'C4 Cally finding: function exercise might fail sporadically due to UI & transaction delays.' On review, we found this to be a wrong analysis. Can you please make this submission invalid. Thanks."