TwentyOne
First Flight #29
Beginner FriendlyGameFiFoundrySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Invalid

Use of `payable.transfer()` in `TwentyOne::endGame` might make it impossible to send a winner's payout

uddercover

Summary

The solidity transfer function only forwards a total of 2300 gas when sending value, this can cause the transfer to fail if the recipient is a smart contract with:

  • A payable fallback which uses more than 2300 gas units.

  • A payable fallback function that needs less than 2300 gas units but is called through proxy, raising the call's gas usage above 2300.

  • Additionally, using more than 2300 gas might be mandatory for some multisig wallets.

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
if (playerWon) {
@> payable(player).transfer(2 ether); // Transfer the prize to the player
emit FeeWithdrawn(player, 2 ether); // Emit the prize withdrawal event
}
}

Vulnerability Details

Proof of Code:

Place the below code into TwentyOneTest in TwentyOne.t.sol

contract TwentyOneTest is Test {
event PlayerWonTheGame(string message, uint256 cardsTotal);
.
.
.
function testTransferFailsIfMoreThan2300GasIsRequired() public {
SmartContractPlayerWithFallbackLogic scpFallbackLogic = new SmartContractPlayerWithFallbackLogic();
SmartContractPlayerWithoutFallbackLogic scpNoFallbackLogic = new SmartContractPlayerWithoutFallbackLogic();
​
//Deal the contracts, including twentyOne to make sure it has eth to send
vm.deal(address(scpFallbackLogic), 1 ether);
vm.deal(address(scpNoFallbackLogic), 1 ether);
vm.deal(address(twentyOne), 2 ether);
​
//Call startGame as scpFallbackLogic
vm.startPrank(address(scpFallbackLogic));
​
uint256 playerHand1 = twentyOne.startGame{value: 1 ether}();
uint256 initialscpFallbackLogicBalance = address(scpFallbackLogic).balance;
​
//scpFallbackLogic calls to compare hands and wins but the transaction reverts
vm.expectRevert();
vm.expectEmit(true, true, false, true);
emit PlayerWonTheGame("Dealer went bust, players winning hand: ", playerHand1);
twentyOne.call();
​
//scpFallback's balance does not increase, prize transfer fails
uint256 finalscpFallbackLogicBalance = address(scpFallbackLogic).balance;
assertEq(finalscpFallbackLogicBalance, initialscpFallbackLogicBalance);
​
vm.stopPrank();
​
//Alternatively, for the contract with no fallback logic
vm.startPrank(address(scpNoFallbackLogic));
​
uint256 playerHand2 = twentyOne.startGame{value: 1 ether}();
uint256 initialscpNoFallbackLogicBalance = address(scpNoFallbackLogic).balance;
​
vm.warp(4);
​
//scpNoFallbackLogic calls to compare hands and wins
vm.expectEmit(true, true, false, true);
emit PlayerWonTheGame("Dealer went bust, players winning hand: ", playerHand2);
twentyOne.call();
​
//scpNoFallbackLogic's balance does increase, transfer is successful
uint256 finalscpNoFallbackLogicBalance = address(scpNoFallbackLogic).balance;
assertGt(finalscpNoFallbackLogicBalance, initialscpNoFallbackLogicBalance);
​
vm.stopPrank();
}
}
​
contract SmartContractPlayerWithFallbackLogic {
uint256 total;
​
fallback() external payable {
total = msg.value;
}
}
​
contract SmartContractPlayerWithoutFallbackLogic {
fallback() external payable {}
}

Impact

Using the transfer function could make it impossible to send value in certain cases. This means that a winner would not receive their payout and would lose their win.

Tools Used

Foundry suite

Recommendations

Using call with its returned boolean checked in combination with re-entrancy guard is highly recommended over transfer.

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
if (playerWon) {
- payable(player).transfer(2 ether); // Transfer the prize to the player
- emit FeeWithdrawn(player, 2 ether); // Emit the prize withdrawal event
+ emit FeeWithdrawn(player, 2 ether); // Emit the prize withdrawal event
+ (bool success, ) = payable(player).call{value: 2 ether}(""); // Transfer the prize to the player
+ require(success, "Transfer failed");
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.