Summary
as stated in the documentation that no second prediction fee is due if any Player desires to change an already paid prediction but actually player can only change their prediction by paying again.
Vulnerability Details
the problem occur because the makePrediction always checks wheter the msg.value equal to prediction fee. and to change predictions, player can only do this by calling the function.
ThePredicter.sol:
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);
}
POC
add the following code to ThePredicter.test.sol:
function test_POCChangePredictionAfterPayingFees() 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.Draw);
vm.expectRevert(abi.encodeWithSelector(ThePredicter__IncorrectPredictionFee.selector));
thePredicter.makePrediction(0, ScoreBoard.Result.First);
vm.stopPrank();
}
then run the command forge test --mt test_POCChangePredictionAfterPayingFees
the result should PASS, indicating the call revert using ThePredicter__IncorrectPredictionFee
Ran 1 test for test/ThePredicter.test.sol:ThePredicterTest
[PASS] test_POCChangePredictionAfterPayingFees() (gas: 188072)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 925.20µs (142.78µs CPU time)
Impact
the protocol are not working as documented, making players pay again if they want to change prediction.
Tools Used
foundry
Recommendations
add a way to checks whether the player already paid or not before checking the msg.value:
ThePredicter.sol:
contract ThePredicter {
.
.
.
address public organizer;
address[] public players;
uint256 public entranceFee;
uint256 public predictionFee;
ScoreBoard public scoreBoard;
mapping(address players => Status) public playersStatus;
+ mapping(address => mapping(uint256 => bool)) public hasPaid;
.
.
.
function makePrediction(uint256 matchNumber, ScoreBoard.Result prediction) public payable {
+ if (!hasPaid[msg.sender][matchNumber]) {
+ if (msg.value != predictionFee) {
+ revert ThePredicter__IncorrectPredictionFee();
+ }
+ }
- if (msg.value != predictionFee) {
- revert ThePredicter__IncorrectPredictionFee();
- }
if (block.timestamp > START_TIME + matchNumber * 68400 - 68400) {
revert ThePredicter__PredictionsAreClosed();
}
scoreBoard.confirmPredictionPayment(msg.sender, matchNumber);
+ hasPaid[msg.sender][matchNumber] = true;
scoreBoard.setPrediction(msg.sender, matchNumber, prediction);
}
after that run the command forge test --mt test_POCChangePredictionAfterPayingFees
the test result should FAIL, indicating the call are not reverted:
Failing tests:
Encountered 1 failing test in test/ThePredicter.test.sol:ThePredicterTest
[FAIL. Reason: call did not revert as expected] test_POCChangePredictionAfterPayingFees() (gas: 239638)
