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

accumulatedFees does Not Reflect Earned Fees if External Call Fails

etijie

Summary

In both _finishGame and _handleTie, the accumulatedFees variable is updated before external ETH transfers are made using .call{value: ...}. If these external calls fail or revert, the entire function reverts — including the accumulatedFees update. This can lead to a situation where:

The protocol fails to retain earned fees.

accumulatedFees does not accurately reflect actual revenue.

An attacker using a malicious fallback (e.g., a Reverter contract) can block fee accumulation and grief the protocol.

Vulnerability Details

_finishGame

accumulatedFees += fee; // Updated before transfer
emit FeeCollected(\_gameId, fee);
​
(bool success,) = \_winner.call{value: prize}("");
require(success, "Transfer failed"); // Reverts all if fails

_handleTie

accumulatedFees += fee;
emit FeeCollected(_gameId, fee);
​
(bool successA,) = game.playerA.call{value: refundPerPlayer}("");
(bool successB,) = game.playerB.call{value: refundPerPlayer}("");
require(successA && successB, "Transfer failed"); // Reverts all

Impact

  1. Inaccurate accounting of fees collected.

  2. Protocol may lose revenue if transaction reverts.

  3. Griefing vector: A player using a contract with a reverting fallback can deliberately cause call failures, invalidating the fee update and breaking prize distribution.

Proof of Concept

contract Reverter {
receive() external payable {
revert("Revert on receive");
}
}

  1. Reverter joins a game and wins or ties.

  2. call{value: ...} to Reverter fails.

  3. Entire function reverts, including accumulatedFees += fee.

Recommendations

Move accumulatedFees += fee after successful ETH transfer:

(bool success,) = _winner.call{value: prize}("");
require(success, "Transfer failed");
​
accumulatedFees += fee;
emit FeeCollected(_gameId, fee);

For _handleTie, check both calls before updating fees:

(bool successA,) = game.playerA.call{value: refundPerPlayer}("");
(bool successB,) = game.playerB.call{value: refundPerPlayer}("");
require(successA && successB, "Transfer failed");
​
accumulatedFees += fee;
emit FeeCollected(_gameId, fee);

Tools Used

Manual review

Updates

Appeal created

m3dython Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

Orphaned ETH due to Unrestricted receive() or Canceled Game

ETH sent directly to the contract via the receive function or after a canceled game becomes permanently locked

Support

FAQs

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