code-423n4 / 2023-12-revolutionprotocol-findings

3 stars 2 forks source link

Wrong comparison of `paymentAmountWei` in `RewardSplits::computeTotalReward` may lead to an user not being able to buy tokens #704

Closed c4-bot-1 closed 9 months ago

c4-bot-1 commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/protocol-rewards/src/abstract/RewardSplits.sol#L41

Vulnerability details

[MEDIUM-1] Wrong comparison of paymentAmountWei in RewardSplits::computeTotalReward may lead to an user not being able to buy tokens

Description:

Since minPurchaseAmount is the minimum amount that can be purchased and maxPurchaseAmount is the maximum amount that can be purchased, the paymentAmountWei in the if statement should be less (<) than minPurchaseAmount and higher (>) than maxPurchaseAmount. Otherwise, if the paymentAmountWei is equal to either minPurchaseAmount or maxPurchaseAmount the function will revert and the total reward would not be computed which will lead to users not being able to buy tokens.

Proof of Concept:

Since RewardSplits::computeTotalReward is called in TokenEmitterRewards::_handleRewardsAndGetValueToSend, and TokenEmitterRewards::_handleRewardsAndGetValueToSend is called in ERC20TokenEmitter::buyToken, the user might end up unable to buy.

41:     if (paymentAmountWei <= minPurchaseAmount || paymentAmountWei >= maxPurchaseAmount) revert INVALID_ETH_AMOUNT();

Tools used:

Manual code review.

Recommended Mitigation:

Change <= to < and >= to >

        if (paymentAmountWei < minPurchaseAmount || paymentAmountWei > maxPurchaseAmount) revert INVALID_ETH_AMOUNT();

Assessed type

call/delegatecall

c4-pre-sort commented 10 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as primary issue

raymondfam commented 10 months ago

Negligibly inconsequential changes.

c4-judge commented 9 months ago

MarioPoneder changed the severity to QA (Quality Assurance)

c4-judge commented 9 months ago

MarioPoneder marked the issue as grade-c