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

0x52 - Adversary can break bounty payouts by adding malicious ERC20 token to bounty #252

Closed
github-actions bot opened this issue Feb 21, 2023 · 0 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

Comments

@github-actions
Copy link

github-actions bot commented Feb 21, 2023

0x52

high

Adversary can break bounty payouts by adding malicious ERC20 token to bounty

Summary

DepositManagerV1#fundBountyToken allows anyone to add any ERC20 token they wish to the bounty. When the bounty is paid it cycles through every token in the bounty list and sends each one to the claimant. An adverary can add a malicious ERC20 tokens that reverts whenever it's transferred between anyone besides themselves. They fund the bounty then whenever anyone attempts to claim the transaction will always revert, making it impossible to claim any bounty.

Vulnerability Detail

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/DepositManager/Implementations/DepositManagerV1.sol#L36-L74

DepositManagerV1#fundBountyToken is a permissionless function that allows any open bounty with any token they want.

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

When a bounty is claimed it loops through all payout tokens and transfers each one.

An adverary can add a malicious ERC20 tokens that reverts whenever it's transferred between anyone besides themselves. They fund the bounty then whenever anyone attempts to claim the transaction will always revert, making it impossible to claim any bounty.

Submitting as high risk because when combined with refund locking methods it will result in all deposited tokens being stuck forever.

Impact

Adversary can permanently disable bounty payouts

Code Snippet

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

Tool used

Manual Review

Recommendation

To maintain the permissionless nature of funding. I recommend implementing secondary bounty-specific whitelists that are set by the bounty issuers. Only allow tokens that appear on either whitelist to fund the bounty. This solves two issues. This allows OpenQ to approve common ERC20 tokens (USDC, USDT, etc) to all bounties while allowing bounty issuers to add uncommon tokens like their protocol token without needing intervention from OpenQ team.

Duplicate of #62

@github-actions github-actions bot added Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue labels Feb 21, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Mar 7, 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
Projects
None yet
Development

No branches or pull requests

1 participant