TwentyOne

First Flight #29
Beginner FriendlyGameFiFoundrySolidity
100 EXP
View results
Submission Details
Severity: high
Invalid

Reentrancy Risk via Internal Function: Potential Exploitation of Ether Transfer in the endGame Function

Summary

The endGame function in the contract contains a transfer to the player if they win the game. Despite being an internal function, it can still trigger a reentrancy attack if called indirectly through external functions that perform an external call, such as the callGame function. If the recipient of the ether (the player) is a contract, that contract could potentially exploit a reentrancy attack, re-entering the contract while the ether is being transferred and manipulating the contract’s state.

Vulnerability Details

The following demonstrates how a reentrancy exploit could be triggered:
<details>
<summary>Code</summary>
```javascript
contract MaliciousPlayer {
address public targetContract;
// Make the constructor payable to accept Ether during contract creation
constructor(address _targetContract) payable {
targetContract = _targetContract;
}
// Receive function: triggered when the contract receives Ether without any data
receive() external payable {
// Optional: Logic for handling plain Ether transfers
}
// Fallback function: triggered when the contract receives Ether with data or when no matching function is found
fallback() external payable {
// Re-enter the contract and trigger the `endGame` function again
(bool success, ) = targetContract.call(abi.encodeWithSignature("call()"));
require(success, "Reentrancy failed");
}
}```
</details>
If this MaliciousPlayer contract is used by a player in the game, and they win, they could trigger multiple reentrancy calls, draining the contract ether supply. The malicious contract would repeatedly call the endGame function while the ether transfer is in progress, causing an issue.

Impact

Although endGame is marked as internal and cannot be called externally, the contract’s public functions may still trigger it. If the external function calls endGame while transferring ether, a malicious player (i.e., one who is a contract with a fallback function) could exploit the transfer mechanism to repeatedly call back into the contract. This leads to unintended behavior, including multiple ether transfers, draining the contract of its funds, or even corrupting the game state by triggering further calls.
This creates a vulnerability in the contract where a malicious actor can siphon funds, causing the contract to become unstable, and rendering the game's payout mechanism unsafe. This undermines the trust of users, as they may lose funds in this way.

Tools Used

FOUNDRY

REMIX

Recommendations

To prevent reentrancy attacks in functions like endGame, consider the following approaches:
1. **Checks-Effects-Interactions Pattern:**
Ensure that state changes (such as clearing player data or updating balances) happen before external calls like transfer(). This minimizes the chances of reentrancy during the transfer process.
Reentrancy Guard:
2. **Use a ReentrancyGuard**
to prevent reentrancy by adding a lock that prevents the function from being entered recursively. This could be implemented using the nonReentrant modifier from OpenZeppelin’s ReentrancyGuard.
3. **Move Transfers After State Updates:**
Ensure that all internal state (such as resetting player hands, updating balances) is completed before the ether transfer, which prevents malicious contracts from exploiting the contract during a transfer.
4. Audit External Interactions:
If the contract interacts with external contracts, ensure that they are properly vetted to avoid the risk of interacting with malicious contracts.
Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

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