Inadequate Handling of Locked Funds Leading to Incorrect Loss Calculation in queueWithdrawal Function

Summary

The queueWithdrawal function is vulnerable due to its failure to verify the availability of the actual funds against the requested withdrawal amount, potentially resulting in the incorrect loss calculation and significant financial loss for users. If the actual fund balance is lower than the amountToBurnAtSettlement at the time of settlement, due to locked or frozen assets not accounted for by the function, users may lose their entitled amount as their owed shares are burned without receiving the proportional amount.

Vulnerability Detail

The root cause of the vulnerability "Inadequate Handling of Locked Funds Leading to Incorrect Loss Calculation in queueWithdrawal Function" in the provided code is that the function queueWithdrawal does not check if the withdrawer has enough funds to cover the withdrawal amount before queuing the withdrawal. This can lead to incorrect loss calculation if the withdrawer does not have sufficient funds to cover the withdrawal amount.

Since the function does not check for the availability of funds before queuing the withdrawal, it may result in a situation where the withdrawer's funds are locked for a withdrawal that cannot be completed due to insufficient funds. This can lead to incorrect loss calculation as the system may assume that the withdrawal will be completed successfully, resulting in inaccurate accounting of the available funds.

The vulnerability in the code lies in the inadequate handling of locked funds, which can lead to incorrect loss calculation in the queueWithdrawal function.

To exploit this vulnerability, an attacker can perform the following steps:

The attacker queues a withdrawal with a large sharesOwed value and a small amountIn value.
Before the withdrawal is processed, the attacker transfers out the majority of their funds from the contract.
When the withdrawal is processed, the contract will calculate the loss based on the amountIn value, which is small, instead of the actual funds the attacker had in the contract.

This will result in the attacker receiving a larger amount than they are entitled to, leading to an incorrect loss calculation.

Proof of Concept (PoC) code:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

contract VulnerableContract {
    uint256 public sharesOwed;
    uint256 public amountIn;
    address public withdrawer;

    function queueWithdrawal(address _withdrawer, uint256 _sharesOwed, uint256 _amountIn) external {
        sharesOwed = _sharesOwed;
        amountIn = _amountIn;
        withdrawer = _withdrawer;
    }

    function exploit() external {
        // Perform steps to exploit the vulnerability
        // 1. Queue a withdrawal with a large sharesOwed and small amountIn
        queueWithdrawal(msg.sender, 1000, 1);

        // 2. Transfer out the majority of funds
        // (code to transfer out funds)

        // 3. Trigger the withdrawal processing
        // (code to trigger withdrawal processing)
    }
}

In the PoC code, an attacker can queue a withdrawal with a large sharesOwed value and a small amountIn value, then transfer out the majority of their funds before triggering the withdrawal processing. This will result in an incorrect loss calculation, allowing the attacker to exploit the vulnerability and receive a larger amount than they are entitled to.

Impact

This could compromise the contract's integrity by failing to deliver promised returns, freezing user funds, or leading to protocol insolvency.

Code Snippet

https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/main/rio-sherlock-audit/contracts/restaking/RioLRTWithdrawalQueue.sol#L130-L145

Tool used

Manual Review

Recommendation

To fix this issue, we need to ensure that the amountIn being added to amountToBurnAtSettlement is deducted from the user's balance before queuing the withdrawal. This way, the locked funds are properly accounted for and the loss calculation will be accurate.

Here is the patched code example:

130       function queueWithdrawal(address withdrawer, address asset, uint256 sharesOwed, uint256 amountIn)
131           external
132           onlyCoordinator
133       {
134           if (sharesOwed == 0) revert NO_SHARES_OWED();
135           uint256 currentEpoch = getCurrentEpoch(asset);
136   
137           EpochWithdrawals storage epochWithdrawals = _getEpochWithdrawals(asset, currentEpoch);
138           epochWithdrawals.sharesOwed += SafeCast.toUint120(sharesOwed);
139           
140           // Deduct the amountIn from the user's balance
141           _deductUserBalance(withdrawer, asset, amountIn);
142   
143           epochWithdrawals.amountToBurnAtSettlement += amountIn;
144   
145           UserWithdrawalSummary storage userSummary = epochWithdrawals.users[withdrawer];
146           userSummary.sharesOwed += SafeCast.toUint120(sharesOwed);
147   
148           emit WithdrawalQueued(currentEpoch, asset, withdrawer, sharesOwed, amountIn);
149       }

In the patched code, a new function _deductUserBalance is called to deduct the amountIn from the user's balance before adding it to amountToBurnAtSettlement. This ensures that the locked funds are properly handled and the loss calculation will be accurate.

sherlock-audit / 2024-02-rio-network-core-protocol-judging

Anubis - Inadequate Handling of Locked Funds Leading to Incorrect Loss Calculation in queueWithdrawal Function #71