Rock Paper Scissors
First Flight #38
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Malicious player contract could always revert and break the internal functions when receiving ETH

0xdef4

Summary

If player A or B is a contract and not an EOA, it can do something malicious (e.g. always revert) when it receives ETH, breaking the functionality of the system.

Vulnerability Details

If player A or B is a contract that has a receive function that always revert when receiving ETH, _finishGame, _handleTie, _cancelGame will always fail when dealing with games created with ETH.

contract AttackerPlayerB {
// When playerB receives ETH it will always revert
receive() external payable {
revert("Malicious player!")
}
}

Impact

In the case when the game was created by ETH, the game will not be able to properly finish and will always revert.

Tools Used

VSCode

Recommendations

We can mitigate this issue by using a pull pattern instead of a push pattern. This method prevents the whole system from crashing, letting the revert only happen in a totally seperate function rather than inside the core game logic.

First we make a seperate mapping storage,

contract RockPaperScissors {
// add a pendingWithdrawals mapping for each player
+ mapping(address => uint256) public pendingWithdrawals;

And in _finishGame update the storage when someone earns money,

function _finishGame(uint256 _gameId, address _winner) internal {
.
.
.
// Dont send funds directly
- (bool success,) = _winner.call{value: prize}("");
- require(success, "Transfer failed");
// record it in data and let the users pull
+ pendingWithdrawals[_winner] += prize;
}

Finally make a seperate withdraw function to withdraw funds,

+ function withdraw() external {
+ uint256 amount = pendingWithdrawals[msg.sender];
+ require(amount > 0, "No funds to withdraw");
​
+ pendingWithdrawals[msg.sender] = 0;
+ (bool success, ) = msg.sender.call{value: amount}("");
+ require(success, "Withdrawal failed");
}

Also, we can enhance security by adding a claim mechanism. For instance, implementing a function like claimFunds(address recipient) lets players transfer funds to a chosen address (e.g., a third party) instead of receiving ETH directly, reducing risks from malicious contracts.

Updates

Appeal created

m3dython Lead Judge 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Denial of Service (DoS) due to Unhandled External Call Revert

Malicious player wins a game using a contract that intentionally reverts when receiving ETH, the entire transaction will fail

m3dython Lead Judge 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Denial of Service (DoS) due to Unhandled External Call Revert

Malicious player wins a game using a contract that intentionally reverts when receiving ETH, the entire transaction will fail

Support

FAQs

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