Project

Summary

The CurrencyManager contract, particularly the addCurrency and removeCurrency functions, has a potential reentrancy vulnerability due to the use of the EnumerableSet data structure in combination with external calls (event emission). While the EnumerableSet itself is not directly vulnerable, the interaction between state changes and external calls (such as emitting events) could enable a reentrancy attack in more complex use cases. Specifically, if a malicious contract calls addCurrency or removeCurrency and re-enters the contract during the event emission or other external interactions, it could cause unexpected behavior or state manipulation.

Vulnerability Details

The functions addCurrency and removeCurrency modify the internal _whitelistedCurrencies set and emit events, but they first modify the state and then emit the event. This sequence of state-changing operations followed by an external call (event emission) opens the door for a reentrancy attack, where an attacker could re-enter the function before the state has been finalized.

• State Changes Before External Calls: The state is modified first (adding or removing the currency from the _whitelistedCurrencies set) and then an event is emitted. If an attacker is able to re-enter the contract (through a malicious contract that overrides fallback/receive functions), they could manipulate the state by re-triggering the same function.

• Reentrancy Vector: The vulnerability exists because:

  1. State-modifying logic (adding/removing currencies) is executed before the external call (event emission).

  2. An attacker could deploy a malicious contract to call addCurrency or removeCurrency, triggering a state change (such as adding or removing a currency).

  3. If the malicious contract has a fallback or receive function, it could call addCurrency or removeCurrency again before the state is completely finalized.

This type of attack could:

• Alter the state of the currency whitelist (e.g., by adding a currency multiple times or removing one that was supposed to remain whitelisted).

• Cause unintended consequences in the whitelisting mechanism, potentially allowing unauthorized currencies to be added or legitimate ones removed.

Impact

• State Manipulation: An attacker could manipulate the whitelisted currencies, either by adding unwanted tokens or removing valid tokens.

• Financial Risk: If the whitelist affects the functionality of financial transactions (e.g., transferring tokens or interacting with other contracts), malicious manipulation of the whitelist could lead to unauthorized financial transactions or the inability to interact with legitimate currencies.

• DAO/Protocol Disruption: If the contract is part of a larger DAO or financial protocol, it could disrupt operations, creating significant vulnerabilities or operational issues.

Tools Used

• Foundry: Used for testing and simulating potential reentrancy attacks by creating a malicious contract that calls the addCurrency and removeCurrency functions and re-enters before the state changes finalize.

• Slither: Automated analysis tool used to detect potential reentrancy vulnerabilities in the smart contract code.

• MythX: A security analysis platform used to scan for known vulnerabilities, including reentrancy and event-related issues.

Recommendations

1. Implement a Reentrancy Guard: To prevent reentrancy attacks, a reentrancy guard should be applied to the functions that modify the state and then emit events. This guard will ensure that the function cannot be entered recursively.

   Example Implementation:

   bool private locked = false;

   ​

   modifier noReentrancy() {

   require(!locked, "ReentrancyGuard: reentrant call");

   locked = true;

   _;

   locked = false;

   }

   ​

   function addCurrency(address currency) external override onlyRole(ADMIN_ROLE) noReentrancy {

   // Existing addCurrency logic here

   }

   ​

   function removeCurrency(address currency) external override onlyRole(ADMIN_ROLE) noReentrancy {

   // Existing removeCurrency logic here

   }

2. Reorder State Changes and Event Emission: Ensure that external calls (like event emission) occur after the state changes have been finalized.

   Example:

   function addCurrency(address currency) external override onlyRole(ADMIN_ROLE) {

   if (currency == address(0))

   revert CurrencyManagerError("Cannot be null address");

   if (_whitelistedCurrencies.contains(currency))

   revert CurrencyManagerError("Already whitelisted");

   _whitelistedCurrencies.add(currency); // State change first

   ​

   emit CurrencyWhitelisted(currency); // External call after state change

   }

   ​

   function removeCurrency(address currency) external override onlyRole(ADMIN_ROLE) {

   if (!_whitelistedCurrencies.contains(currency))

   revert CurrencyManagerError("Not whitelisted");

   _whitelistedCurrencies.remove(currency); // State change first

   ​

   emit CurrencyRemoved(currency); // External call after state change

   }

3. Consider Using OpenZeppelin's ReentrancyGuard: OpenZeppelin provides a ReentrancyGuard contract that could simplify the implementation of reentrancy protection across multiple functions.

   Example:

   import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

   ​

   contract CurrencyManager is ICurrencyManager, AccessControl, ReentrancyGuard {

   // Contract logic here

   }

4. Audit External Contracts: Ensure that any contracts interacting with CurrencyManager are secure, particularly focusing on malicious fallback or receive functions that could trigger reentrancy attacks.

Proof of Concept for Reentrancy Attack

Overview:

The vulnerability allows an attacker to manipulate the whitelist of accepted currencies by repeatedly calling the addCurrency or removeCurrency function through reentrancy. This is facilitated by the state-modifying logic followed by an event emission, which can be exploited to trigger a recursive call before the state change is finalized.

Actors:

• Attacker: A malicious user who deploys a contract that re-enters the addCurrency or removeCurrency function during the event emission process.

• Victim: The CurrencyManager contract and the system relying on it, which may suffer from unauthorized changes to the whitelist of currencies.

• Protocol: The CurrencyManager contract, which is responsible for managing the whitelisted currencies.

Working Test Case (PoC):

Explanation:

1. Line 8-10: The malicious contract accepts the CurrencyManager contract address during deployment.

2. Line 13-15: The attacker triggers the addCurrency function, which will modify the state.

3. Line 18-21: The receive function is triggered upon an external call, allowing the attacker to re-enter the addCurrency function before the state change is finalized, potentially adding the same currency multiple times.

Example Code of the Affected Function:

Example of How It Was Infected:

• The contract modifies the _whitelistedCurrencies set (state change) before emitting an event. This ordering is susceptible to reentrancy attacks by a malicious contract with a receive function.

Example of How to Repair:

• Implement a Reentrancy Guard modifier to prevent reentrancy.

• Reorder the external call (event emission) to ensure it happens after the state modification.