[WP-L11] `_createDeposit()` `_lockedUntil` should be upper-bounded

[code423n4]

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/Vault.sol#L425-L430

if (_lockedUntil == 0) _lockedUntil = block.timestamp + minLockPeriod;
else
    require(
        _lockedUntil >= block.timestamp + minLockPeriod,
        "Vault: lock time is too small"
    );

Given that _lockedUntil is not upper-bounded, if the user passes an excessively large value mistakenly, their funds can be locked for an unreasonably period of time.

Considering the severity of the impact, we suggest adding a maxLockPeriod.

Recommendation

if (_lockedUntil == 0) _lockedUntil = block.timestamp + minLockPeriod;
else
    require(
        _lockedUntil >= block.timestamp + minLockPeriod,
        "Vault: lock time is too small"
    );
    require(
        _lockedUntil < block.timestamp + maxLockPeriod,
        "Vault: lock time is too small"
    );

[0xBRM]

Not a bug.

[dmvt]

This is desired functionality apparently. I have to admit I agree with the warden's suggestion, but also with the sponsor that this does not represent a bug. No funds are at risk and I'm not even sure this require would be worth the gas.