Rock Paper Scissors

🪲 Finding Title

Critical Access Control Flaw in setAdmin() Enables Full Fee Drain

🧩 Summary

The RockPaperScissors.sol contract defines a single adminAddress role which is responsible for managing key protocol operations, including setting a new admin and withdrawing accumulated protocol fees. However, the access control mechanism used by setAdmin() lacks sufficient governance protections, allowing a privileged user to transfer admin rights without oversight.

If an attacker is able to become or impersonate the current admin (through a logic bug or deployment misconfiguration), they can assign themselves as the new admin and immediately drain all collected fees using withdrawFees().

🔍 Finding Description

The vulnerability originates from the self-governed nature of setAdmin():

• The only requirement is that msg.sender == adminAddress.

• No ownership check, multi-sig approval, or time lock is enforced.

• If an attacker becomes admin (via initial misconfiguration or another bug), they can call setAdmin() to take over the protocol.

Once admin, the attacker can invoke withdrawFees():

This function allows the current admin to withdraw all protocol fees. Together, these two functions create a critical privilege escalation vector.

💥 Impact

• Unauthorized admin takeover: Any actor who obtains admin rights gains full control over the protocol.

• Complete fee theft: Accumulated ETH in the contract can be drained without detection or restriction.

• No recovery: Once admin rights are transferred, there is no built-in way to revert it.

• No governance: No owner, DAO, or multi-sig involvement is enforced, leaving the protocol permanently exposed.

📌 Vulnerable Code (Combined)

🧪 Proof of Concept (PoC)

See: AccessControlPoC_Final.t.sol

This PoC simulates a privileged attacker changing the admin address to their own, then draining all collected fees:

The test passes, confirming that all funds were successfully sent to the attacker's address.

✅ Recommendation

To prevent this vulnerability:

1. 🔐 Replace adminAddress-based control with an Ownable or AccessControl system.

2. 🔐 Make setAdmin() callable only by owner() or through a governance contract.

3. 🕒 Optionally introduce a delay or timelock mechanism before role transitions take effect.

4. 🔍 Add an event for admin transfer with old and new address to improve transparency.

Recommended pattern:

📌 Severity Justification

Severity: Critical

✅ Conclusion

The lack of robust access control in setAdmin() allows any actor with admin privileges to permanently seize control over the protocol and withdraw all funds. This type of unchecked privilege escalation is a critical risk and must be addressed using established governance and role management practices.