Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Anybody can cancel orders made by RubiconRouter #117

Closed
code423n4 opened this issue May 27, 2022 · 1 comment
Closed

Anybody can cancel orders made by RubiconRouter #117

code423n4 opened this issue May 27, 2022 · 1 comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly 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-rubicon/blob/main/contracts/RubiconRouter.sol#L440

Vulnerability details

Anybody can cancel an offer made by [another user using RoubiconRouter].

Impact

If a user created an offer which wasn't fully filled, anybody (including bots) can swoop in, cancel it, and get the user's funds.

Proof of Concept

Anybody can create an offer using offerForETH. The router does not save the offer's originator.
Then by calling 'cancelForETH', which does not check who called it, anybody can sweep the user's funds.

Recommended Mitigation Steps

Few options are there -

  • Consider saving in the router the offer's originator - although that ruins the router's statelessness
  • Consider adding a parameter to RubiconMarket's offer function (\overload) that will allow passing who can cancel the offer
  • Consider removing the functionality altogether
  • Consider adding a comment warning the user of the risk of using this functionality
  • Consider the futility of constantly trying to chase happiness
@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels May 27, 2022
code423n4 added a commit that referenced this issue May 27, 2022
@bghughes bghughes added the duplicate This issue or pull request already exists label Jun 4, 2022
@bghughes
Copy link
Collaborator

bghughes commented Jun 4, 2022

Duplicate of #17

@bghughes bghughes marked this as a duplicate of #17 Jun 4, 2022
@bghughes bghughes closed this as completed Jun 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

2 participants