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

3 stars 2 forks source link

`ERC20TokenEmitter.buyToken()` will revert if the purchase amount equals `minPurchaseAmount` or `maxPurchaseAmount` due to the flawed validation. #675

Closed c4-bot-4 closed 9 months ago

c4-bot-4 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

Impact

computeTotalReward() in RewardSplits contract is invoked by ERC20TokenEmitter.buyToken() to compute total rewards before buying ERC20Votes tokens. The RewardSplits contract defines minPurchaseAmount and maxPurchaseAmount. However, the invalid validation in RewardSplits.computeTotalReward() leads to revert the function call of ERC20TokenEmitter.buyToken() when attempting to purchase tokens for the specified minPurchaseAmount or maxPurchaseAmount.

Proof of Concept

minPurchaseAmount and maxPurchaseAmount determine the minimum and maximum amounts necessary to acquire ERC20Votes tokens as specified in the RewardSplits contract.

File: packages/protocol-rewards/src/abstract/RewardSplits.sol

23:    uint256 public constant minPurchaseAmount = 0.0000001 ether;
24:    uint256 public constant maxPurchaseAmount = 50_000 ether;

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

This information indicates to users that they have the option to purchase ERC20Votes tokens with a minimum of 0.0000001 ether and a maximum of 50,000 ether. The acquisition of tokens can be facilitated through the buyToken() function within the ERC20TokenEmitter contract.

File: packages/revolution/src/ERC20TokenEmitter.sol

152:    function buyToken(
153:        address[] calldata addresses,
154:        uint[] calldata basisPointSplits,
155:        ProtocolRewardAddresses calldata protocolRewardsRecipients
156:    ) public payable nonReentrant whenNotPaused returns (uint256 tokensSoldWad) {
157:        //prevent treasury from paying itself
158:        require(msg.sender != treasury && msg.sender != creatorsAddress, "Funds recipient cannot buy tokens");
159:
160:        require(msg.value > 0, "Must send ether");
161:        // ensure the same number of addresses and bps
162:        require(addresses.length == basisPointSplits.length, "Parallel arrays required");
163:
164:        // Get value left after protocol rewards
165: @>     uint256 msgValueRemaining = _handleRewardsAndGetValueToSend(
166:            msg.value,
167:            protocolRewardsRecipients.builder,
168:            protocolRewardsRecipients.purchaseReferral,
169:            protocolRewardsRecipients.deployer
170:        );

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/ERC20TokenEmitter.sol#L152C5-L170C11

Refer to line number 165 in the provided code, where _handleRewardsAndGetValueToSend() is called. In the corresponding code snippet for _handleRewardsAndGetValueToSend(), observe line number 18, where computeTotalReward() is invoked.

File: packages/protocol-rewards/src/abstract/TokenEmitter/TokenEmitterRewards.sol

12:    function _handleRewardsAndGetValueToSend(
13:        uint256 msgValue,
14:        address builderReferral,
15:        address purchaseReferral,
16:        address deployer
17:    ) internal returns (uint256) {
18: @>      if (msgValue < computeTotalReward(msgValue)) revert INVALID_ETH_AMOUNT();

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/protocol-rewards/src/abstract/TokenEmitter/TokenEmitterRewards.sol#L12C5-L18C82

In the following code snippet, observe the computeTotalReward() function. Pay attention to line number 41, where validation for the payment or purchase amount is performed using <= minPurchaseAmount and >= maxPurchaseAmount. Notably, these comparisons exclude the values of minPurchaseAmount and maxPurchaseAmount.

File: packages/protocol-rewards/src/abstract/RewardSplits.sol

40:    function computeTotalReward(uint256 paymentAmountWei) public pure returns (uint256) {
41: @>     if (paymentAmountWei <= minPurchaseAmount || paymentAmountWei >= maxPurchaseAmount) revert INVALID_ETH_AMOUNT();

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

Should an individual attempt to acquire tokens with an amount equal to minPurchaseAmount or maxPurchaseAmount, the transaction will encounter a revert. The validation dictates that the minimum permissible amount for purchase is minPurchaseAmount + 1 Wei, while the maximum allowable amount is maxPurchaseAmount - 1 Wei.

Tools Used

Manual Review

Recommended Mitigation Steps

Update the validation in the computeTotalReward() as follows.

     function computeTotalReward(uint256 paymentAmountWei) public pure returns (uint256) {
-        if (paymentAmountWei <= minPurchaseAmount || paymentAmountWei >= maxPurchaseAmount) revert INVALID_ETH_AMOUNT();
+        if (paymentAmountWei < minPurchaseAmount || paymentAmountWei > maxPurchaseAmount) revert INVALID_ETH_AMOUNT();

Assessed type

Invalid Validation

c4-pre-sort commented 10 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #8

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #160

c4-judge commented 9 months ago

MarioPoneder marked the issue as not a duplicate

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