Summary
PuppyRaffle::enterRaffle is too computationally intensive making it so for every call it spends more and more gas, making the whole purpose of this protocol unrealistic.
Vulnerability Details
The enterRaffle function in the PuppyRaffle contract performs an highly intensive computation each time anyone calls on it to enter an ongoing lottery which makes it so that for every entry into an ongoing contest their would be an increase in gas fee for the next person that would want to enter the contest, making the lottery totally unfair.
Impact
I would be showing the amount of gas used for entering the PuppyRaffle contract with varying number of players in the newPlayers array and from that you would be able to deduct how serious this bug 🐛 is.
Calling the PuppyRaffle::enterRaffle with 10 players in the newPlayers array we are passing in as an argument to the function
code
modifier ManyPlayersEntered1() {
address[] memory players = new address[](10);
for(uint256 player = 0; player < players.length; player++ ){
players[player] = address(player + 1);
}
puppyRaffle.enterRaffle{value: entranceFee * players.length}(players);
_;
}
function testMakesEntranceMoreExpensiveDueToHeavyEntryComputation() public ManyPlayersEntered1 {
}
output
Running 1 test for test/PuppyRaffleTest.t.sol:PuppyRaffleTest
[PASS] testMakesEntranceMoreExpensiveDueToHeavyEntryComputation() (gas: 307963)
Traces:
[307963] PuppyRaffleTest::testMakesEntranceMoreExpensiveDueToHeavyEntryComputation()
├─ [291155] PuppyRaffle::enterRaffle{value: 10000000000000000000}([0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000002, 0x0000000000000000000000000000000000000003, 0x0000000000000000000000000000000000000004, 0x0000000000000000000000000000000000000005, 0x0000000000000000000000000000000000000006, 0x0000000000000000000000000000000000000007, 0x0000000000000000000000000000000000000008, 0x0000000000000000000000000000000000000009, 0x000000000000000000000000000000000000000A])
│ ├─ emit RaffleEnter(newPlayers: [0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000002, 0x0000000000000000000000000000000000000003, 0x0000000000000000000000000000000000000004, 0x0000000000000000000000000000000000000005, 0x0000000000000000000000000000000000000006, 0x0000000000000000000000000000000000000007, 0x0000000000000000000000000000000000000008, 0x0000000000000000000000000000000000000009, 0x000000000000000000000000000000000000000A])
│ └─ ← ()
└─ ← ()
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 18.39ms
conclusion
calling the PuppyRaffle::enterRaffle with 10 players in the newPlayers argument we are passing in cost 307963 gas
Calling the PuppyRaffle::enterRaffle with 20 players in the newPlayers array we are passing in as an argument to the function
code
modifier ManyPlayersEntered2() {
address[] memory players = new address[](20);
for(uint256 player = 0; player < players.length; player++ ){
players[player] = address(player + 1);
}
puppyRaffle.enterRaffle{value: entranceFee * players.length}(players);
_;
}
function testMakesEntranceMoreExpensiveDueToHeavyEntryComputation() public ManyPlayersEntered2 {
}
output
Running 1 test for test/PuppyRaffleTest.t.sol:PuppyRaffleTest
[PASS] testMakesEntranceMoreExpensiveDueToHeavyEntryComputation() (gas: 656774)
Traces:
[656774] PuppyRaffleTest::testMakesEntranceMoreExpensiveDueToHeavyEntryComputation()
├─ [637413] PuppyRaffle::enterRaffle{value: 20000000000000000000}([0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000002, 0x0000000000000000000000000000000000000003, 0x0000000000000000000000000000000000000004, 0x0000000000000000000000000000000000000005, 0x0000000000000000000000000000000000000006, 0x0000000000000000000000000000000000000007, 0x0000000000000000000000000000000000000008, 0x0000000000000000000000000000000000000009, 0x000000000000000000000000000000000000000A, 0x000000000000000000000000000000000000000b, 0x000000000000000000000000000000000000000C, 0x000000000000000000000000000000000000000d, 0x000000000000000000000000000000000000000E, 0x000000000000000000000000000000000000000F, 0x0000000000000000000000000000000000000010, 0x0000000000000000000000000000000000000011, 0x0000000000000000000000000000000000000012, 0x0000000000000000000000000000000000000013, 0x0000000000000000000000000000000000000014])
│ ├─ emit RaffleEnter(newPlayers: [0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000002, 0x0000000000000000000000000000000000000003, 0x0000000000000000000000000000000000000004, 0x0000000000000000000000000000000000000005, 0x0000000000000000000000000000000000000006, 0x0000000000000000000000000000000000000007, 0x0000000000000000000000000000000000000008, 0x0000000000000000000000000000000000000009, 0x000000000000000000000000000000000000000A, 0x000000000000000000000000000000000000000b, 0x000000000000000000000000000000000000000C, 0x000000000000000000000000000000000000000d, 0x000000000000000000000000000000000000000E, 0x000000000000000000000000000000000000000F, 0x0000000000000000000000000000000000000010, 0x0000000000000000000000000000000000000011, 0x0000000000000000000000000000000000000012, 0x0000000000000000000000000000000000000013, 0x0000000000000000000000000000000000000014])
│ └─ ← ()
└─ ← ()
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.16ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)
conclusion
calling the PuppyRaffle::enterRaffle with 20 players in the newPlayers argument we are passing in cost 656774 gas
Tools Used
Foundry
Recommendations
I would recommend the protocol use a mapping for storing the address of every member that entered into the Raffle an address to bool mapping would do the trick mapping(address => bool) . this is would make sure that we have a constant time search when we are trying to verify that there is no duplicate address in the data structure that saves every players that have entered the raffle.
