code-423n4 / 2022-01-sandclock-findings

0 stars 0 forks source link

_lockedUntil is not deterministic and does not have an upper boundary #175

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

pauliax

Vulnerability details

Impact

function sponsor accepts a parameter of timestamp _lockedUntil, but I do not it is very convenient because of the deterministic nature of blockchain. When you send the tx you cannot be sure when it will be mined, so it may result in reverted txs in case _lockedUntil becomes below MIN_SPONSOR_LOCK_DURATION while waiting to be mined in the mempool. I think it would be better to accept the lock period, and then add it to the block.timestamp in the function.

Another problem is that _lockedUntil does not have an upper boundary, so a fat finger error can cost a deposit locked forever.

Recommended Mitigation Steps

Consider replacing _lockedUntil with lock period and introducing a reasonable upper limit, e.g. MAX_SPONSOR_LOCK_DURATION.