DoS of entire protocol with two EOA accounts
By exploiting a flaw in the structure of players, a malicious user can create a DoS of the whole protocol
Description
-
Due to the way checking of duplicate players is implemented in the enterRaffle function, refunding from two different addresses creates a DoS of the enterRaffle function.
-
That is the case because refunding from two different addresses creates two identical addresses (zero address) in the array, which should never be in a state with two identical addresses.
-
That flaw in the enterRaffle function can be accidentally exploited by users, but it can also be leveraged by a malicious user to cause even greater damage to the protocol and permanently block the raffle contract.
-
Users can independently refund and block the enterRaffle function until the selectWinner is called, but can also block enterRaffle and selectWinner, and by that also withdrawFees (in the case there is also a third user in the raffle) if these two users happen to be the first two users who entered the raffle and refunded, because of the require statement of minimum length of players array in selectWinner function.
-
A malicious user can target an attack to be first after the new raffle round starts (could be done by listening to the selectWinner function call in mempool and backrunning it). An attacker can call enterRaffle with two addresses, call the refund function from both addresses, and create a permanent DoS of the whole protocol. The whole attack can be done in one tx if he executes it from the smart contract utilizing account abstraction.
-
Recommended Mitigation
Instead of relying only on the array for the players structure, it is recommended to track players with a combination of the array and mapping.
Lead Judging Commences
[H-04] `PuppyRaffle::refund` replaces an index with address(0) which can cause the function `PuppyRaffle::selectWinner` to always revert
## Description `PuppyRaffle::refund` is supposed to refund a player and remove him from the current players. But instead, it replaces his index value with address(0) which is considered a valid value by solidity. This can cause a lot issues because the players array length is unchanged and address(0) is now considered a player. ## Vulnerability Details ```javascript players[playerIndex] = address(0); @> uint256 totalAmountCollected = players.length * entranceFee; (bool success,) = winner.call{value: prizePool}(""); require(success, "PuppyRaffle: Failed to send prize pool to winner"); _safeMint(winner, tokenId); ``` If a player refunds his position, the function `PuppyRaffle::selectWinner` will always revert. Because more than likely the following call will not work because the `prizePool` is based on a amount calculated by considering that that no player has refunded his position and exit the lottery. And it will try to send more tokens that what the contract has : ```javascript uint256 totalAmountCollected = players.length * entranceFee; uint256 prizePool = (totalAmountCollected * 80) / 100; (bool success,) = winner.call{value: prizePool}(""); require(success, "PuppyRaffle: Failed to send prize pool to winner"); ``` However, even if this calls passes for some reason (maby there are more native tokens that what the players have sent or because of the 80% ...). The call will thankfully still fail because of the following line is minting to the zero address is not allowed. ```javascript _safeMint(winner, tokenId); ``` ## Impact The lottery is stoped, any call to the function `PuppyRaffle::selectWinner`will revert. There is no actual loss of funds for users as they can always refund and get their tokens back. However, the protocol is shut down and will lose all it's customers. A core functionality is exposed. Impact is high ### Proof of concept To execute this test : forge test --mt testWinnerSelectionRevertsAfterExit -vvvv ```javascript function testWinnerSelectionRevertsAfterExit() public playersEntered { vm.warp(block.timestamp + duration + 1); vm.roll(block.number + 1); // There are four winners. Winner is last slot vm.prank(playerFour); puppyRaffle.refund(3); // reverts because out of Funds vm.expectRevert(); puppyRaffle.selectWinner(); vm.deal(address(puppyRaffle), 10 ether); vm.expectRevert("ERC721: mint to the zero address"); puppyRaffle.selectWinner(); } ``` ## Recommendations Delete the player index that has refunded. ```diff - players[playerIndex] = address(0); + players[playerIndex] = players[players.length - 1]; + players.pop() ```
Rewards breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Judging
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsRewards Distribution
The contest is complete and the rewards are being distributed.