#45405 [SC-Insight] Insufficient Documentation for Governance-Controlled Functions and Critical Parameters in 'CoreVaultManager.sol'

Submitted on May 14th 2025 at 00:34:54 UTC by @rusalka711 for Audit Comp | Flare | FAssets

  • Report ID: #45405

  • Report Type: Smart Contract

  • Report severity: Insight

  • Target: https://github.com/flare-labs-ltd/fassets/blob/main/docs/ImmunefiScope.md

  • Impacts:

Description

Brief/Intro

Governance functions like updateSettings, addPreimageHashes, and account management utilities lack explanations for parameter constraints, role definitions, and system dependencies. Missing context increases the risk of misconfiguration during governance actions, potentially destabilizing escrow mechanics or access control.

Vulnerability Details

Affected Components:

  1. addTriggeringAccounts/removeTriggeringAccounts:

    • No definition of "triggering accounts" or their privileges.

    • Example:

      function addTriggeringAccounts(address[] calldata _triggeringAccounts)  
      // What permissions do these accounts have? How do they interact with `triggerInstructions`?  
  2. updateCustodianAddress:

    • Undocumented impact of custodian changes on escrow lifecycle.

      function updateCustodianAddress(string calldata _custodianAddress)  
      // How does this affect existing escrows or payment instructions?  
  3. updateSettings:

    • Lack of rationale for constraints (e.g., _escrowEndTimeSeconds < 1 days).

    • No explanation of parameter interplay:

      require(_escrowEndTimeSeconds < 1 days, "invalid end time");  
      // Why enforce <1 day? How does this align with UTC cycles?  
  4. addPreimageHashes:

    • Missing context on preimage hash lifecycle (e.g., usage in escrows, redemption).

      require(_preimageHashes[i] != bytes32(0), "invalid preimage hash");  
      // What is a preimage hash? How is it generated/used?  

Impact Details

Category: Insight - Documentation Improvements

Impact Analysis:

  • No Direct Exploit: Functions operate as intended.

  • Operational Risks:

    • Incorrect escrow configurations due to misunderstood time parameters.

    • Invalid preimage hashes disrupting redemption workflows.

    • Privilege escalation if triggering accounts are misconfigured.

References

  • Code File: CoreVaultManager.sol (link: https://github.com/flare-labs-ltd/fassets/blob/main/contracts/assetManager/implementation/CoreVaultManager.sol)

  • Key Functions:

    • updateSettings (Lines 504-520).

    • addPreimageHashes (Lines 527-538).

    • Role management functions (Lines 449-460, 467-478 and 485-494).

Proof of Concept

Proof of Concept

1. updateSettings Function

Improved Documentation:

/**
 * @notice Updates critical protocol parameters.  
 * @param _escrowEndTimeSeconds End time for escrows (in seconds within a UTC day, e.g., 79200 = 22:00 UTC). Must be < 86400.  
 * @param _escrowAmount Amount to escrow per batch. Set to 0 to disable escrows.  
 * @param _minimalAmount Minimum reserve balance retained in the vault after escrowing to prevent insolvency.  
 * @param _fee Per-request fee (deducted from available funds). Must be > 0.  
 * @dev Misconfiguration (e.g., setting _minimalAmount = 0) may risk vault insolvency.  
 */  
function updateSettings(...)  

2. addTriggeringAccounts and removeTriggeringAccounts

Improved Documentation:

/**
 * @notice Authorizes addresses to trigger escrow/transfer processing.  
 * @param _triggeringAccounts List of addresses to grant triggering privileges.  
 * @dev Triggering accounts can call `triggerInstructions()`. Restrict to trusted actors.  
 * @dev Batch size limited to 50 addresses to avoid gas overflows.  
 */  
function addTriggeringAccounts(...)  

/**
 * @notice Revokes triggering privileges from addresses.  
 * @param _triggeringAccounts List of addresses to remove.  
 * @dev Removed accounts can no longer call `triggerInstructions()`.  
 */  
function removeTriggeringAccounts(...)  

3. addPreimageHashes Function

Improved Documentation:

/**
 * @notice Adds cryptographic commitments (preimage hashes) for escrow redemptions.  
 * @param _preimageHashes List of unique, non-zero hashes (use keccak256(secret) for secure generation).  
 * @dev Preimage hashes must never be reused. Leaked secrets allow unauthorized redemptions.  
 */  
function addPreimageHashes(...)  

4. updateCustodianAddress Function

Improved Documentation:

/**
 * @notice Updates the custodian address for future escrow redemptions.  
 * @param _custodianAddress New custodian address (must be non-empty).  
 * @dev Changes apply only to new escrows. Existing escrows retain the previous custodian.  
 */  
function updateCustodianAddress(...)  

5. addTriggeringAccounts Function

Improved Documentation:

/**  
 * @notice Grants triggering privileges to specified addresses.  
 * @param _triggeringAccounts List of addresses to authorize.  
 * @dev  
 * - Authorized accounts can call `triggerInstructions()`, which processes escrows and transfers.  
 * - Batch size must be ≤ 50 addresses to avoid gas limits.  
 * - Triggering accounts are highly privileged; audit and restrict to trusted entities.  
 * @emit TriggeringAccountAdded: Successfully added addresses.  
 */  
function addTriggeringAccounts(address[] calldata _triggeringAccounts) external onlyGovernance {  
    // ...  
}  

6. removeTriggeringAccounts Function

Improved Documentation:

/**  
 * @notice Revokes triggering privileges from specified addresses.  
 * @param _triggeringAccounts List of addresses to revoke.  
 * @dev  
 * - Removed addresses can no longer call `triggerInstructions()`.  
 * - Use this to mitigate risks from compromised or deprecated accounts.  
 * @emit TriggeringAccountRemoved: Successfully removed addresses.  
 */  
function removeTriggeringAccounts(address[] calldata _triggeringAccounts) external onlyGovernance {  
    // ...  
}  

Was this helpful?