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

Auction.sol#settleAuction() addBounty with a fake token could potentially disrupt settleAuction() #82

Open
code423n4 opened this issue Sep 20, 2021 · 1 comment
Labels
1 (Low Risk) Assets are not at risk. State handling, function incorrect as to spec, issues with comments bug Warden finding

Comments

@code423n4
Copy link
Contributor

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2021-09-defiProtocol/blob/main/contracts/contracts/Auction.sol#L102

Anyone can call addBounty() to add a bounty with any token. If we assume that the frontend will always pass all the bountyIDs of active bounties to settleAuction(), then a malicious user can disrupt settleAuction() by addBounty with a fake token that always reverts when calling transfer().

Impact

Auction bonder wont be able to settleAuction(). The malicious user and all other holders of the basket token can belifits from the burn of the auction bond.

Proof of Concept

  1. Create a fake token that allways reverts at transfer();
  2. addBounty with the fake token and any amount;
  3. Calling settleAuction with bountyIDs including the fake token bounty will always fail.

Recommended Mitigation Steps

Whitelist bounty tokens in smart contract or frontend.

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Warden finding labels Sep 20, 2021
code423n4 added a commit that referenced this issue Sep 20, 2021
@GalloDaSballo
Copy link
Collaborator

By adding a malicious token as bounty, we can make the settleAuction transaction revert.
Simple mitigation is to not claim that bounty.

Generally speaking this can be viewed as medium severity because it's dependent on an external condition.
But personally I think mitigation and likelihood are so low that I'll downgrade to low.

Will mark as low, because all it takes to avoid this type of griefing is to remove the bounty from the list, the fact that the frontend may not has no relevance for this contest

@GalloDaSballo GalloDaSballo added 1 (Low Risk) Assets are not at risk. State handling, function incorrect as to spec, issues with comments and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Dec 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 (Low Risk) Assets are not at risk. State handling, function incorrect as to spec, issues with comments bug Warden finding
Projects
None yet
Development

No branches or pull requests

2 participants