DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: low
Valid

PerpetualVault withdrawals are affected by global parameter updates

Summary

If the protocol changes the lockTime, it should only apply to new deposits and not affect existing ones. If the lock period is extended, users might be forced to keep their funds locked for a longer time than originally expected, preventing planned timely withdrawals during losses or profits. Conversely, if the lock period is shortened, early withdrawals from older deposits could disrupt trading strategies, leading to forced liquidations or premature position closures.

Impact

  • Extended Lock Period: Users unable to withdraw during desired periods, potentially leading to forced losses or reduced profits.

  • Reduced Lock Period: Unplanned withdrawals might disrupt vault strategies, causing position liquidations or premature position closures.

Recommendations

  • Implement logic to ensure lockTime changes only apply to future deposits.
    add a uint256 lockTime variable to the DepositInfo struct
    and check for lock durations using each deposits respective lockTime and not the global parameter.

function withdraw(address recipient, uint256 depositId) public payable nonReentrant {
_noneFlow();
flow = FLOW.WITHDRAW;
flowData = depositId;
if (recipient == address(0)) {
revert Error.ZeroValue();
}
//Use local lockTime parameter
if (depositInfo[depositId].timestamp + depositInfo[depositId].lockTime >= block.timestamp) {
revert Error.Locked();
}
[...]
}
Updates

Lead Judging Commences

n0kto Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Appeal created

riceee Submitter
3 months ago
n0kto Lead Judge
3 months ago
n0kto Lead Judge 3 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_changing_lockTime_impact_previous_depositors

Likelihood: Low, when admin changes lockTime setting. Impact: Informational/Low, it will change the lockTime for previous depositors, forcing them to wait longer than expected or allowing them to withdraw earlier. This is indeed a strange implementation and is not specified in the documentation. It deserves a low.

Support

FAQs

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