Part 2

Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Submission Details
Severity: medium
Invalid

Missing access control in AssetSwapPath.configure()

Summary

The AssetSwapPath.configure() function allows configuring swap paths and strategies for an asset. However, this function lacks proper access control, making it vulnerable to unauthorized calls. A malicious actor could exploit this vulnerability to modify critical swap configurations, leading to incorrect trades, financial losses, or system instability.


Vulnerability Details

Code Analysis

The configure() function is defined as follows:

function configure(
Data storage self,
bool enabled,
address[] memory assets,
uint128[] memory dexSwapStrategyIds
) internal {
self.enabled = enabled;
self.assets = assets;
self.dexSwapStrategyIds = dexSwapStrategyIds;
}

Key Observations:

  1. No Access Control : The function does not enforce any restrictions on who can call it. If exposed externally or called improperly, it could allow unauthorized users to modify critical swap configurations.

  2. Critical State Modification : The function modifies the enabled flag, assets, and dexSwapStrategyIds fields of the AssetSwapPath.Data struct. These fields are essential for determining swap strategies and paths.

  3. Potential Misconfiguration : An attacker could set invalid or malicious swap paths, such as pointing to non-existent or malicious contracts, leading to incorrect trades or losses.

Attack Scenario

  1. An attacker identifies that the configure() function is callable without restrictions.

  2. The attacker calls the configure() function for a valid asset, setting enabled = false or providing invalid assets and dexSwapStrategyIds.

  3. The swap strategy for the asset is disabled or misconfigured, causing trades involving the asset to fail or behave incorrectly.

  4. Users attempting to trade the asset experience failures or unexpected behavior, leading to financial losses or system instability.


Impact

  • Incorrect Trades : Misconfigured swap paths could lead to incorrect trades, resulting in financial losses for users.

  • Denial of Service : Disabling the swap strategy (enabled = false) could prevent trades involving the asset, disrupting the system.

  • Loss of Trust : Users may lose confidence in the protocol due to unreliable or malicious swap configurations.


Tools Used

  1. Manual Code Review : Analyzed the configure() function and its interactions with the AssetSwapPath.Data struct.

  2. Slither : Static analysis tool used to identify missing access control and potential misconfigurations.

  3. MythX : Security analysis platform used to verify vulnerabilities in the smart contract.


Recommendations

Short-Term Fix

Add proper access control to the configure() function to ensure only authorized entities can call it. For example:

modifier onlyAuthorized() {
require(msg.sender == authorizedAddress, "Unauthorized");
_;
}
function configure(
Data storage self,
bool enabled,
address[] memory assets,
uint128[] memory dexSwapStrategyIds
) internal onlyAuthorized {
self.enabled = enabled;
self.assets = assets;
self.dexSwapStrategyIds = dexSwapStrategyIds;
}

Replace authorizedAddress with the appropriate address or role (e.g., admin, governance).

Long-Term Fix

  1. Role-Based Access Control (RBAC) : Implement an RBAC system using libraries like OpenZeppelin's AccessControl to manage permissions for sensitive functions.

  2. Input Validation : Add validation checks to ensure the assets and dexSwapStrategyIds arrays are valid and consistent.

    require(assets.length == dexSwapStrategyIds.length, "Mismatched lengths");
    require(assets[0] == expectedAccessAsset, "Invalid first asset");
  3. Event Logging : Emit events whenever the configure() function is called to provide transparency and enable monitoring.

    event AssetSwapPathConfigured(address indexed asset, bool enabled, address[] assets, uint128[] dexSwapStrategyIds);
    function configure(
    Data storage self,
    bool enabled,
    address[] memory assets,
    uint128[] memory dexSwapStrategyIds
    ) internal onlyAuthorized {
    require(assets.length == dexSwapStrategyIds.length, "Mismatched lengths");
    require(assets[0] == expectedAccessAsset, "Invalid first asset");
    self.enabled = enabled;
    self.assets = assets;
    self.dexSwapStrategyIds = dexSwapStrategyIds;
    emit AssetSwapPathConfigured(self.asset, enabled, assets, dexSwapStrategyIds);
    }
Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

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

Give us feedback!