Summary
Ask fees for already paid round, if user has to update his choice.
Vulnerability Details
function makePrediction(
uint256 matchNumber,
ScoreBoard.Result prediction
) public payable {
if (msg.value != predictionFee) {
revert ThePredicter__IncorrectPredictionFee();
}
if (block.timestamp > START_TIME + matchNumber * 68400 - 68400) {
revert ThePredicter__PredictionsAreClosed();
}
scoreBoard.confirmPredictionPayment(msg.sender, matchNumber);
scoreBoard.setPrediction(msg.sender, matchNumber, prediction);
}
In above function, there is no check if user has already paid for given round. It will require user to pay fees again and again even if they want to update there choice for the round, they already paid for.
This will be loss of user funds, as they will be paying unnecessarily.
POC
In existing test suite, add following test
function test_FeesNeedsToPaidEveryTimeForPredictionEvenToChangeCurrentRoundChoice() public {
vm.startPrank(stranger);
vm.warp(1);
vm.deal(stranger, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(organizer);
vm.warp(2);
thePredicter.approvePlayer(stranger);
vm.stopPrank();
vm.startPrank(stranger);
thePredicter.makePrediction{value: 0.0001 ether}(
0,
ScoreBoard.Result.First
);
vm.expectRevert(abi.encodeWithSelector(
ThePredicter__IncorrectPredictionFee.selector
));
thePredicter.makePrediction{value: 0 ether}(
1,
ScoreBoard.Result.Second
);
vm.stopPrank();
}
run the following command in terminal forge run --mt test_FeesNeedsToPaidEveryTimeForPredictionEvenToChangeCurrentRoundChoice and following results will be printed out as output.
[⠊] Compiling...
[⠃] Compiling 1 files with Solc 0.8.20
[⠊] Solc 0.8.20 finished in 1.89s
Compiler run successful!
Ran 1 test for test/ThePredicter.test.sol:ThePredicterTest
[PASS] test_FeesNeedsToPaidEveryTimeForPredictionEvenToChangeCurrentRoundChoice() (gas: 188078)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 9.03ms (793.21µs CPU time)
Ran 1 test suite in 159.14ms (9.03ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
Loss of user funds
Tools Used
Manual Review
Recommendations
Add a check, if user has already paid for particular round, don't ask fees again.
+ mapping (address => mapping(uint256=> bool) isPaid;
function makePrediction(
uint256 matchNumber,
ScoreBoard.Result prediction
) public payable {
+ if(![msg.sender][matchNumber]){
if (msg.value != predictionFee) {
revert ThePredicter__IncorrectPredictionFee();
}
+ [msg.sender][matchNumber] = true;
+ scoreBoard.confirmPredictionPayment(msg.sender, matchNumber);
}
if (block.timestamp > START_TIME + matchNumber * 68400 - 68400) {
revert ThePredicter__PredictionsAreClosed();
}
- scoreBoard.confirmPredictionPayment(msg.sender, matchNumber);
scoreBoard.setPrediction(msg.sender, matchNumber, prediction);
}
