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

Lack of Token Recovery Mechanism

0x68

Summary

The RockPaperScissors contract has no mechanism to recover tokens that are accidentally sent to the contract address. This includes both the internal WinningToken and any external ERC20 or ERC721 tokens that might be transferred to the contract by mistake.

Vulnerability Details

The contract handles WinningToken as part of its normal game operations, but lacks any function that would allow the admin or owner to recover:

  1. Excess WinningToken tokens that accumulate in the contract due to canceled games, timeouts, or other edge cases

  2. Any other ERC20/ERC721 tokens that users might accidentally send to the contract address

The only token management implemented is the minting of new tokens as rewards. There is no functionality to:

  • Withdraw tokens that are not part of active games

  • Recover tokens in case of contract migration or upgrade

  • Handle tokens sent erroneously to the contract

Unlike ETH, which can be withdrawn through the withdrawFees function, tokens have no recovery path:

function withdrawFees(uint256 _amount) external {
require(msg.sender == adminAddress, "Only admin can withdraw fees");
// ETH can be recovered, but no equivalent for tokens
// ...
}

Impact

The impact of this vulnerability is moderate as it doesn't directly compromise game mechanics or user funds in active games. However, it does create situations where:

  1. Tokens become permanently locked in the contract

  2. Users who make mistakes cannot recover their assets

  3. In case of contract deprecation, accumulated tokens become unrecoverable

Over time, this could result in significant value being locked in the contract with no way to recover it.

Tools Used

Manual code review

Recommendations

Implement token recovery functions that allow the admin to recover tokens that aren't part of active games:

/**
* @notice Allows admin to recover any ERC20 tokens sent to the contract by mistake
* @param _tokenAddress The address of the token to recover
* @param _amount The amount of tokens to recover
*/
function recoverERC20(address _tokenAddress, uint256 _amount) external {
require(msg.sender == adminAddress, "Only admin can recover tokens");
// If recovering the game token, ensure we don't take tokens needed for active games
if (_tokenAddress == address(winningToken)) {
uint256 activeGameTokens = _calculateActiveGameTokens();
uint256 contractBalance = winningToken.balanceOf(address(this));
require(contractBalance - activeGameTokens >= _amount, "Cannot recover tokens needed for active games");
}
IERC20(_tokenAddress).transfer(adminAddress, _amount);
}
​
/**
* @dev Helper function to calculate tokens needed for active games
* @return Total number of tokens that are part of active games
*/
function _calculateActiveGameTokens() internal view returns (uint256) {
// Implementation depends on game logic and state tracking
// Should count tokens that belong to active games only
// ...
}

Additionally, consider implementing a similar function for ERC721 tokens and native ETH recovery for amounts that exceed the accumulatedFees balance.

Updates

Appeal created

m3dython Lead Judge about 2 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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