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

jkoppel - Up to 99 tokens of each type will be left in a tiered percentage bounty contest after all tiers have been claimed #97

Closed
github-actions bot opened this issue Feb 21, 2023 · 4 comments
Labels
Low/Info A valid Low/Informational severity issue Non-Reward This issue will not receive a payout Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@github-actions
Copy link

jkoppel

medium

Up to 99 tokens of each type will be left in a tiered percentage bounty contest after all tiers have been claimed

Summary

Rounding errors cause some tokens to be left in a a tiered percentage bounty contest after all tiers have been claimed

Vulnerability Detail

Tier payouts are divided by 100 and rounded down.

Consider the extreme case where there are 100 tiers, each with 1% of the prize, and there is a total of 99 tokens in the funding pool. Then each tier winner will be awarded 0 tokens, and the 99 tokens will be stuck in the bounty.

Impact

This will cause some amount of token up to 99 to be stuck in the bounty. If the token has 18 decimal places as is standard, this will be very low impact. If it does not, it can be significant. For example, Gemini USD only has 2 decimal places, meaning $0.99 can be stuck in every bounty that uses Gemini USD.

One might think this can be worked around by having the issuer call setPayableSchedule, assign 100% of funds to a brand-new tier, and then set themselves as the winner of that tier. However, this does not work, as the amount to be claimed is based on fundingTotals, which is computed by closeCompetition, which can only be called once.

The funds can still be refunded however, unless the separately-reported attack disabling refunds has been carried out. See https://github.com/sherlock-audit/2023-02-openq-jkoppel/issues/3 .

Code Snippet

Here is the offending code, from TieredPercentageBountyV1.claimTiered, https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/TieredPercentageBountyV1.sol#L104 .

uint256 claimedBalance = (payoutSchedule[_tier] *
            fundingTotals[_tokenAddress]) / 100;

The division by 100 rounds down, causing the token loss.

Tool used

Manual Review

Recommendation

In closeCompetition, precompute the amount of token to be provided to each tier, and ensure they add up to the funding total.

@github-actions github-actions bot added the Medium A valid Medium severity issue label Feb 21, 2023
@FlacoJones
Copy link

Will fix by removing tiered percentage for now

@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

@IAm0x52
Copy link
Collaborator

IAm0x52 commented Mar 5, 2023

Should be low. Loss of rewards is tiny and that tiny amount isn't stuck because funder can withdraw (assuming they would even care about such a small amount.

@hrishibhat
Copy link
Contributor

Agree with Lead Watson comment.

@sherlock-admin sherlock-admin added Low/Info A valid Low/Informational severity issue Non-Reward This issue will not receive a payout and removed Medium A valid Medium severity issue labels Mar 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Low/Info A valid Low/Informational severity issue Non-Reward This issue will not receive a payout 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

4 participants