Summary

RockPaperScissors contract implements custom ownership/admin system instead of using the ownable (or AccessControl) library from OpenZeppelin.

Vulnerability Details

While it seems there are no direct vulnerabilities in the implementation, there is no advantage to implement its custom ownership management inside the game contract instead of using the ownable library from OpenZeppelin.
See https://docs.openzeppelin.com/contracts/5.x/api/access#Ownable

Using this library brings several advantage:

• Battle tested and audited contract

• Implements ERC-173:

  • Standardized functions to manage ownership

  • Standardized functions emitted when there is a transferred of ownership.
    For example, the custom function setAdmindoes not emit an event while the OpenZeppelin implementation will emit the event: emit OwnershipTransferred(oldOwner, newOwner);

• Reduce the cost of an audit since there is no need to include the OpenZeppelin library in the scope of the contract (save precious money)

Impact

• Add unnecessary complexity to the contract

• Does not implement ERC-173 which could make easier to monitor admin/owner change

Additionnaly, the implementation does not respect the following specification:
"Admin: The protocol administrator who can update timeout parameters and withdraw accumulated fees
Contract Owner: Initially the deployer of the contract, capable of setting a new admin"

Here we can see that there are two different roles: the contract owner and the admin, but only the admin is set at deployment.
This is also why I have increases my initial severity from Low to medium

Tools Used

Manual analysis

Recommendations

Use the library ownable from OpenZeppelin
OR AccessControl if the author want several separate roles