Part 2

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

Reentrancy risk in CreditDelegation.clear()

Summary

The CreditDelegation.clear() function deletes all fields of the CreditDelegation.Data struct when a vault's credit share is fully undelegated from a market. However, this function lacks proper access control, making it vulnerable to unauthorized calls. A malicious actor could exploit this vulnerability to clear critical state data, leading to loss of important information and potential denial of service.


Vulnerability Details

Code Analysis

The clear() function is defined as follows:

function clear(Data storage self) internal {
delete self.vaultId;
delete self.marketId;
delete self.weight;
delete self.valueUsd;
delete self.lastVaultDistributedRealizedDebtUsdPerShare;
delete self.lastVaultDistributedUnrealizedDebtUsdPerShare;
delete self.lastVaultDistributedUsdcCreditPerShare;
delete self.lastVaultDistributedWethRewardPerShare;
}

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 clear critical state data.

  2. State Corruption : Deleting all fields of the CreditDelegation.Data struct removes essential information about the delegation, including vaultId, marketId, and other accounting values. This could lead to permanent loss of state and disrupt the system's functionality.

  3. Reentrancy Risk : If the function interacts with external contracts or state updates, it could be vulnerable to reentrancy attacks.

Attack Scenario

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

  2. The attacker calls the clear() function for a valid vaultId and marketId pair.

  3. All fields of the CreditDelegation.Data struct are deleted, causing the loss of critical state data.

  4. The system loses track of the delegation, leading to incorrect accounting and potential financial losses.


Impact

  • Critical State Loss : Deletion of critical state data (vaultId, marketId, weight, etc.) leads to permanent loss of information.

  • Denial of Service : Markets relying on the delegation data will fail to operate correctly, disrupting the system.

  • Financial Losses : Incorrect accounting due to missing delegation data could result in improper debt or reward distributions, affecting users and the protocol.


Tools Used

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

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

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


Recommendations

Short-Term Fix

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

modifier onlyAuthorized() {
require(msg.sender == authorizedAddress, "Unauthorized");
_;
}
function clear(Data storage self) internal onlyAuthorized {
delete self.vaultId;
delete self.marketId;
delete self.weight;
delete self.valueUsd;
delete self.lastVaultDistributedRealizedDebtUsdPerShare;
delete self.lastVaultDistributedUnrealizedDebtUsdPerShare;
delete self.lastVaultDistributedUsdcCreditPerShare;
delete self.lastVaultDistributedWethRewardPerShare;
}

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

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. Event Logging : Emit events whenever the clear() function is called to provide transparency and enable monitoring.

    event CreditDelegationCleared(uint128 vaultId, uint128 marketId);
    function clear(Data storage self) internal onlyAuthorized {
    emit CreditDelegationCleared(self.vaultId, self.marketId);
    delete self.vaultId;
    delete self.marketId;
    delete self.weight;
    delete self.valueUsd;
    delete self.lastVaultDistributedRealizedDebtUsdPerShare;
    delete self.lastVaultDistributedUnrealizedDebtUsdPerShare;
    delete self.lastVaultDistributedUsdcCreditPerShare;
    delete self.lastVaultDistributedWethRewardPerShare;
    }
  3. Reentrancy Guard : If the function interacts with external contracts or state updates, use a reentrancy guard to prevent reentrancy attacks.

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!