Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

incorrect operator used in buyOption() function #56

Closed
code423n4 opened this issue May 11, 2022 · 3 comments
Closed

incorrect operator used in buyOption() function #56

code423n4 opened this issue May 11, 2022 · 3 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate This issue or pull request already exists

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L224

Vulnerability details

Impact

In Cally.sol the buyOption() function requires that the msg.value is greater than or equal to the premium when it should always be greater and not equal since the premium is the base fee to buy the option. The msg.value should naturally always be higher than the base fee when buying an option.

Proof of Concept

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L224

Tools Used

Manual code review

Recommended Mitigation Steps

-    require(msg.value >= premium, "Incorrect ETH amount sent");
+    require(msg.value > premium, "Incorrect ETH amount sent");
@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels May 11, 2022
code423n4 added a commit that referenced this issue May 11, 2022
@outdoteth outdoteth added the help wanted Extra attention is needed label May 15, 2022
@outdoteth
Copy link
Collaborator

It should be equal not greater

require(msg.value == premium);

Not sure if this qualifies as an issue because it gives the incorrect solution but does identify an issue

@outdoteth
Copy link
Collaborator

reference issue: #84

@outdoteth outdoteth added the duplicate This issue or pull request already exists label May 15, 2022
@HardlyDifficult
Copy link
Collaborator

Yes, we'll still consider this a valid submission -- the more important point is they identified an issue. Keeping this as a dupe

@HardlyDifficult HardlyDifficult removed the help wanted Extra attention is needed label May 22, 2022
@HickupHH3 HickupHH3 mentioned this issue Jun 8, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

3 participants