Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

usmannk - Bounties can be rendered nonfunctional by depositing and refunding an NFT. #329

Closed
github-actions bot opened this issue Feb 21, 2023 · 2 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@github-actions
Copy link

github-actions bot commented Feb 21, 2023

usmannk

high

Bounties can be rendered nonfunctional by depositing and refunding an NFT.

Summary

The Atomic, Tiered Fixed, and Tiered Percentage bounty types support awarding NFTs as a bounty reward. When claims are made, the NFT is transferred to the claimant. However, there are no checks on whether or not the NFT has previously been refunded to the depositor. If an NFT is deposited into a bounty and refunded, every subsequent claim will attempt to claim that NFT and revert.

A malicious issuer can take advantage of this by:

  • Attacker funds bounties with large amounts of USDC and long expiration periods.
  • Attacker adds an NFT to the bounty or (each bounty tier) with expiration time of 1 second.
  • In the next block the attacker refunds the NFT to themselves.

From this point onwards, any claims to the bounty will revert. Once the expiration period is over, the attacker will have gotten victims to do the off-chain bounty tasks for no payout.

Vulnerability Detail

When a claim is made to any bounty/bounty tier the following code is run (it is duplicated for each of the 3 vulnerable bounty types).

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/ClaimManager/Implementations/ClaimManagerV1.sol#L150-L165

The claimNft function is defined once. It does not check if the bounty contract still owns the NFT.

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/BountyCore.sol#L125-L136

Impact

Loss of funds to claimants in the amount of the entire claim.

Although this may be "only" classified as "theft of unclaimed yield", I believe it is high severity because claimants must do real off-chain work (like completing a Github Pull Request) before submitting a claim. They are, in effect, being robbed of the work done.

Code Snippet

Tool used

Manual Review

Recommendation

Check if an NFT has been previously refunded before attempting to transfer it away.

Duplicate of #263

@github-actions github-actions bot added the High A valid High severity issue label Feb 21, 2023
@FlacoJones FlacoJones added Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels Feb 23, 2023
@FlacoJones
Copy link

will fix by removing erc721 funding functionality

@FlacoJones
Copy link

OpenQDev/OpenQ-Contracts#113 and OpenQDev/OpenQ-Contracts#114 and OpenQDev/OpenQ-Contracts#116 effectively nullify this exploit

@sherlock-admin sherlock-admin added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Reward A payout will be made for this issue and removed High A valid High severity issue labels Mar 7, 2023
@sherlock-admin sherlock-admin added High A valid High severity issue and removed Medium A valid Medium severity issue labels Mar 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

3 participants