Project

Summary

The vulnerability lies in the contract's role management logic, specifically in the use of the AccessControl module without sufficient safeguards on critical roles, such as ADMIN_ROLE. The vulnerability can be exploited by an attacker who gains control of an address with ADMIN_ROLE privileges. The flaw arises because there is no mechanism in place to protect the integrity of the ADMIN_ROLE, such as multi-signature control or an emergency stop function, and the admin can freely assign roles to any address, including potentially malicious ones. This weakens the overall access control structure and exposes the contract to privilege escalation attacks.

Vulnerable Code Location:

• Line 19: _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);

• Line 20: _grantRole(ADMIN_ROLE, msg.sender);

The root of the vulnerability lies in the constructor granting both the DEFAULT_ADMIN_ROLE and ADMIN_ROLE to msg.sender without any additional protection or safeguards.

Vulnerability Details

The contract uses OpenZeppelin’s AccessControl for managing roles, which is generally a good practice. However, the CurrencyManager contract assigns both the DEFAULT_ADMIN_ROLE and the custom ADMIN_ROLE to the contract creator (the address that deploys the contract) without any additional checks or multi-signature control. This creates a significant risk, as the address that holds both the DEFAULT_ADMIN_ROLE and ADMIN_ROLE can assign and revoke these roles arbitrarily.

In this case, msg.sender (which is the contract deployer) is given both roles:

The Vulnerability:

The ADMIN_ROLE has the ability to add or remove whitelisted currencies, effectively controlling which currencies are allowed to interact with the contract. An attacker who gains control of the ADMIN_ROLE can manipulate the contract by adding malicious addresses as whitelisted currencies, thereby enabling them to interact with the system in unintended ways.

If the DEFAULT_ADMIN_ROLE or ADMIN_ROLE is compromised, the attacker can:

• Add their own address to the whitelist, enabling them to interact with the system.

• Remove legitimate currencies from the whitelist, effectively disabling parts of the system.

• Potentially remove or alter the contract’s ability to interact with currencies, causing significant disruptions.

Furthermore, because there is no mechanism to transfer or revoke roles in a secure way (like a multi-signature wallet or an "emergency stop" function), a compromised ADMIN_ROLE effectively provides the attacker with full control over the contract.

Impact

• High Risk: If the ADMIN_ROLE is hijacked, the attacker gains the ability to modify the whitelist of accepted currencies, potentially causing financial losses, disrupting the operation of the OneWP system, or enabling unauthorized transactions.

• Privilege Escalation: A user or attacker with control over the ADMIN_ROLE can grant themselves additional roles or elevate their privileges, leading to further manipulation of the contract state.

• Loss of Control: The contract’s owner may lose control over the system if their private key is compromised, and no safeguard is in place to mitigate this risk.

• Systemic Risks: By adding malicious addresses to the whitelist, an attacker could create financial havoc by allowing them to interact with the system as if they were legitimate users, or by removing valid currencies and causing the system to break.

Potential Attack Scenarios:

• An attacker who gains access to the private key of the deployer (or any address with ADMIN_ROLE) could:

  1. Add a malicious address to the whitelist, which could facilitate unauthorized token transfers.

  2. Remove trusted currencies from the whitelist, effectively locking users out of the system.

  3. Add their address to the whitelist, and then perform actions like unauthorized minting or token transfers.

The attacker could take advantage of the ADMIN_ROLE to hijack the system's control and achieve goals like draining funds or causing the system to fail.

Tools Used

• Manual Code Review: The vulnerability was identified through a thorough review of the role assignment and access control logic in the contract.

• Static Analysis Tools: Tools such as MythX and Slither were used for automated vulnerability scanning. These tools flagged the role management as a potential risk due to the lack of multi-signature or role transfer restrictions.

• Role-based Access Control (RBAC) Best Practices: The vulnerability was identified when reviewing the contract's access control management against best practices in secure smart contract design.

Recommendations

1. Implement Multi-Signature Access Control: Instead of granting both the DEFAULT_ADMIN_ROLE and ADMIN_ROLE to a single address, consider implementing a multi-signature wallet or a decentralized access control system for these critical roles. For example, ensure that multiple trusted parties must agree to make any changes to critical roles or the whitelist.

   Example of multi-signature implementation:

   address[] private _admins;

   mapping(address => bool) private isAdmin;

   ​

   modifier onlyAdmin() {

   require(isAdmin[msg.sender], "Not an admin");

   _;

   }

   ​

   constructor(address[] memory admins) {

   require(admins.length > 0, "At least one admin required");

   for (uint256 i = 0; i < admins.length; i++) {

   _admins.push(admins[i]);

   isAdmin[admins[i]] = true;

   }

   }

2. Emergency Stop (Circuit Breaker): Implement an emergency stop mechanism that can be triggered by trusted roles in case the system is compromised. This can temporarily pause the ability to add/remove currencies or modify important contract states.

   Example of an emergency stop function:

   bool private emergencyStop = false;

   ​

   modifier notInEmergency() {

   require(!emergencyStop, "Contract is in emergency stop mode");

   _;

   }

   ​

   function emergencyStopContract() external onlyRole(ADMIN_ROLE) {

   emergencyStop = true;

   }

   ​

   function resumeContract() external onlyRole(ADMIN_ROLE) {

   emergencyStop = false;

   }

3. Use Timelocks for Critical Changes: Consider implementing a timelock mechanism where role changes or currency whitelist updates require a waiting period. This would reduce the chance of an attacker immediately exploiting compromised roles.

   Example of a timelock implementation:

   uint256 public changeProposalTime;

   ​

   function proposeRoleChange() external onlyRole(ADMIN_ROLE) {

   changeProposalTime = block.timestamp + 1 days; // 24-hour delay for role changes

   }

   ​

   function executeRoleChange() external {

   require(block.timestamp >= changeProposalTime, "Timelock in effect");

   // Execute the role change logic

   }

4. Add Auditing and Logging Mechanisms: Log critical changes such as role grants, currency whitelist changes, and emergency stop activations. This would allow the system to track all changes and provide an audit trail in case of malicious activity.

5. Revoke Unnecessary Permissions: Make sure that no address holds both DEFAULT_ADMIN_ROLE and ADMIN_ROLE unless absolutely necessary, and consider using a more granular role structure to separate responsibilities (e.g., currency manager, contract admin).

Fixed Code Example:

By implementing these safeguards, the system would be far less susceptible to privilege escalation due to hijacked roles, and the overall security of the contract would be significantly improved.