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

rTokenTrader#distributeTokenToBuy could be bypassed during setDistribution by purposefully providing too little gas #205

Open
c4-bot-3 opened this issue Aug 19, 2024 · 0 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working 🤖_primary AI based primary recommendation sufficient quality report This report is of sufficient quality

Comments

@c4-bot-3
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/Distributor.sol#L61-L71

Vulnerability details

Impact

Users who stand to gain more distribution after the change could prevent distribution so they gain an unfair distribution.

Proof of Concept

Distributor.sol#L61-L71

function setDistribution(address dest, RevenueShare calldata share) external governance {
    // solhint-disable-next-line no-empty-blocks
    try main.rsrTrader().distributeTokenToBuy() {} catch {}
    // solhint-disable-next-line no-empty-blocks
    try main.rTokenTrader().distributeTokenToBuy() {} catch {}

    _setDistribution(dest, share);

    RevenueTotals memory revTotals = totals();
    _ensureSufficientTotal(revTotals.rTokenTotal, revTotals.rsrTotal);
}

When setting/updating distribution shares, the distributor attempts to distribute all pending tokens before making any changes. This ensures that tokens are distributed fairly according to the pre-update distributions.

When using try-catch, the default gas sent is 63/64 of the transaction gas. Although distribution is set by governance proposal, anyone can carry out the final execution. The result is that an interested party could provide just little enough gas to cause the distribution to trigger a OOG error but still have enough gas to finish updating the revenue shares. After the new distribution is in place they can distribute the token and benefit unfairly.

Tools Used

Manual review

Recommended Mitigation Steps

The try-catch pattern in setDistribution and setDistributions should be updated with the pattern that reverts on OOG errors.

Assessed type

Other

@c4-bot-3 c4-bot-3 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Aug 19, 2024
c4-bot-9 added a commit that referenced this issue Aug 19, 2024
@c4-bot-12 c4-bot-12 added the 🤖_primary AI based primary recommendation label Aug 19, 2024
howlbot-integration bot added a commit that referenced this issue Aug 20, 2024
@howlbot-integration howlbot-integration bot added the sufficient quality report This report is of sufficient quality label Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working 🤖_primary AI based primary recommendation sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

2 participants