No Enforcement of the Documented `lockTime > 1 day` Constraint
Summary:
Although the documentation indicates a lock time of at least 1 day, there is no on-chain enforcement preventing the owner from setting the lock time to an extremely high value (or even zero)—effectively contradicting the stated design and risking indefinite lockouts or zero lockups.
RootCause & Where It Happens
The contract’s documentation (contest references) describes:
“lockTime > 1 day”
However, the contract itself allows the owner to set any arbitrary value for lockTime:
There is no require() or other check to ensure _lockTime remains above 1 day (86400 seconds), or even to prevent it from being set to extremely large values. This means:
The owner can set
lockTimeto0, negating the withdrawal lock entirely.The owner can set
lockTimeto a massive value (e.g., millions of seconds), effectively preventing users from withdrawing for months or years.
Issue PROOF ->
Users deposit funds under the assumption that they cannot withdraw for 1 day after depositing, but can certainly do so afterward.
The contract code does not actually enforce a 1-day minimum. An unscrupulous or compromised owner could set
lockTime = 10 years, rendering all deposits essentially stuck until that time elapses.This breaks the “Withdrawal Locks” invariant stated in the documentation. The doc says “lockTime > 1 day,” but the code never enforces that minimum.
Impact
High severity from a user perspective: If the protocol is supposed to ensure only a short (e.g., 1-day) lock time, but the owner can set an indefinite or arbitrarily long lock time, depositors could be locked out of their funds far longer than they anticipated.
Also, if the system relies on a minimum lock time for safety, the owner could set it to
0, allowing immediate deposit-and-withdraw “gaming” or front-running.
Recommended Remediation
-
Add a Require Check in
setLockTime()function setLockTime(uint256 _lockTime) external onlyOwner {// For example, enforce 1 day <= lockTime <= 30 daysrequire(_lockTime >= 1 days, "lockTime < 1 day");// Optionally require an upper bound if the protocol forbids extremely long lockslockTime = _lockTime;} -
Align Documentation With Reality
If the protocol truly wants flexible lock times, make it clear in docs that the owner can set any value.
Otherwise, enforce the documented minimum.
Lead Judging Commences
Admin is trusted / Malicious keepers
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. Keepers are added by the admin, there is no "malicious keeper" and if there is a problem in those keepers, that's out of scope. ReadMe and known issues states: " * System relies heavily on keeper for executing trades * Single keeper point of failure if not properly distributed * Malicious keeper could potentially front-run or delay transactions * Assume that Keeper will always have enough gas to execute transactions. There is a pay execution fee function, but the assumption should be that there's more than enough gas to cover transaction failures, retries, etc * There are two spot swap functionalies: (1) using GMX swap and (2) using Paraswap. We can assume that any swap failure will be retried until success. " " * Heavy dependency on GMX protocol functioning correctly * Owner can update GMX-related addresses * Changes in GMX protocol could impact system operations * We can assume that the GMX keeper won't misbehave, delay, or go offline. " "Issues related to GMX Keepers being DOS'd or losing functionality would be considered invalid."
Admin is trusted / Malicious keepers
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. Keepers are added by the admin, there is no "malicious keeper" and if there is a problem in those keepers, that's out of scope. ReadMe and known issues states: " * System relies heavily on keeper for executing trades * Single keeper point of failure if not properly distributed * Malicious keeper could potentially front-run or delay transactions * Assume that Keeper will always have enough gas to execute transactions. There is a pay execution fee function, but the assumption should be that there's more than enough gas to cover transaction failures, retries, etc * There are two spot swap functionalies: (1) using GMX swap and (2) using Paraswap. We can assume that any swap failure will be retried until success. " " * Heavy dependency on GMX protocol functioning correctly * Owner can update GMX-related addresses * Changes in GMX protocol could impact system operations * We can assume that the GMX keeper won't misbehave, delay, or go offline. " "Issues related to GMX Keepers being DOS'd or losing functionality would be considered invalid."
Appeal created
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.
Prize pool breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.