Part 2

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

Uncontrolled Vault Access: Missing Return Statement in loadLive Function Enables Unauthorized Access to Disabled Vaults

Summary

A critical security vulnerability has been identified in the loadLive function where the vault storage variable is not explicitly returned, potentially allowing unauthorized access to disabled vaults. This oversight could compromise the security controls intended to prevent operations on disabled vaults.

Vulnerability Details

function loadLive(uint128 vaultId) internal view returns (Data storage vault) {
vault = loadExisting(vaultId);
if (!vault.isLive) {
revert Errors.VaultIsDisabled(vaultId);
}
} // Missing return statement

Root Cause

The root cause of this vulnerability is a missing return statement in the loadLive function. While the function declares a return type of Data storage vault, it fails to explicitly return the vault variable after checking the isLive condition. This creates a potential security gap in the vault access control mechanism.

Impact

The impact of this vulnerability includes:

  • Potential unauthorized access to disabled vaults

  • Bypass of security controls

  • Compromise of vault state management

  • Risk of unintended operations on disabled vaults


i think this should be classified as Medium or High depending on how loadLive is used in the protocol. If the missing return value leads to broken functionality, incorrect access control, or unintended reverts, it's a serious issue

Tools Used

  • Solidity compiler (0.8.25)

  • Static analysis tools

  • Code review methodology

Proof of Concept

Here's a test case demonstrating the vulnerability:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;
import {Vault} from "../Vault.sol";
contract VaultTest {
Vault vault;
function testLoadLiveDisabledVault() public {
// Create a vault and disable it
uint128 vaultId = 1;
vault.create(Vault.CreateParams({
depositFee: 0,
redeemFee: 0,
vaultId: vaultId,
depositCap: 1000,
withdrawalDelay: 0,
indexToken: address(0),
engine: address(0),
collateral: Vault.Data({/* ... */})
}));
vault.update(Vault.UpdateParams({
vaultId: vaultId,
depositCap: 1000,
withdrawalDelay: 0,
isLive: false,
lockedCreditRatio: 0
}));
// This should fail but might succeed due to missing return
try vault.loadLive(vaultId) {
// If we reach here, the vulnerability is present
assert(false, "Should have reverted");
} catch Error(string memory reason) {
assertEq(reason, "VaultIsDisabled(uint128)");
}
}
}

Mitigation

To fix this vulnerability, add an explicit return statement:

function loadLive(uint128 vaultId) internal view returns (Data storage vault) {
vault = loadExisting(vaultId);
if (!vault.isLive) {
revert Errors.VaultIsDisabled(vaultId);
}
return vault; // Add this line
}

Recommendations

  1. Implement the fix immediately as this is a critical security vulnerability

  2. Add explicit return statements to all functions with declared return types

  3. Review all similar functions for missing return statements

  4. Consider adding linter rules to catch missing return statements

  5. Update testing suite to verify proper function returns

Severity

High

This vulnerability has been rated as high severity due to its potential to bypass security controls and enable unauthorized access to disabled vaults, which could lead to significant security breaches and financial losses.

Updates

Lead Judging Commences

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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