Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Lack of error handling in `withdraw` function can hinder debugging and user experience, as users are left without detailed information on why their transaction was unsuccessful in `VaultControllerStrategy.sol`

safdie

Summary

he withdraw function of the VaultDepositController contract contains inadequate error handling, reverting with a generic InsufficientTokensUnbonded error. This lack of specificity makes it difficult for users and developers to understand the underlying issue when a transaction fails, leading to potential misuse or unnecessary confusion.

Vulnerability Details

he withdraw function of the VaultDepositController contract contains inadequate error handling, reverting with a generic InsufficientTokensUnbonded error. This lack of specificity makes it difficult for users and developers to understand the underlying issue when a transaction fails, leading to potential misuse or unnecessary confusion.

if (!fundFlowController.claimPeriodActive() || _amount > totalUnbonded)
revert InsufficientTokensUnbonded();

PoC:

  1. A user interacts with the withdraw function.

  2. The claim period is inactive (e.g., set to false).

  3. The user attempts to withdraw a certain amount of tokens.
    When the withdrawal attempt is made, the user will receive the generic error message InsufficientTokensUnbonded, without clarification on whether the failure was due to the inactive claim period or the requested amount being greater than the available unbonded tokens.

Below is a Hardhat test scenario that proves the vulnerability in the withdraw function:

const { expect } = require("chai");
const { ethers } = require("hardhat");
​
describe("VaultDepositController", function () {
let vaultDepositController;
let fundFlowController;
​
before(async function () {
// Deploy the contracts needed for the test
const FundFlowController = await ethers.getContractFactory("FundFlowController");
fundFlowController = await FundFlowController.deploy();
const VaultDepositController = await ethers.getContractFactory("VaultDepositController");
vaultDepositController = await VaultDepositController.deploy(/* constructor parameters */);
});
​
it("should revert with insufficient tokens error without details", async function () {
// Set the claim period to inactive
await fundFlowController.setClaimPeriodActive(false);
​
// Attempt to withdraw
await expect(vaultDepositController.withdraw(100)).to.be.revertedWith("InsufficientTokensUnbonded");
});
});

Impact

The impact of inadequate error handling is significant. Users cannot discern whether their transaction failed due to an inactive claim period or an insufficient balance, leading to confusion and potential loss of trust in the contract. Developers may also struggle to diagnose issues without detailed error messages, resulting in increased debugging time and costs.

Tools Used

Manual review.

Recommendations

To mitigate this issue, the error handling in the withdraw function should be enhanced. Instead of reverting with a generic error, it should specify the exact reason for failure, such as:

if (!fundFlowController.claimPeriodActive()) {
revert("Claim period is inactive");
}
if (_amount > totalUnbonded) {
revert("Requested amount exceeds available unbonded tokens");
}

Additionally, consider emitting events before reverting to provide a log of the attempted actions and reasons for failure.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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