code-423n4 / 2021-12-amun-findings

0 stars 0 forks source link

Lock time is dependent on the average block time #282

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

Czar102

Vulnerability details

Impact

Function BasketFacet::getLock(...) checks the lock based on the block number, so the time of the lock is dependent on average block time.

Average block time doesn't have to be maintained by the protocol and is a subject to changes. Furthermore, the Difficulty Bomb will definitely change the average block time.

Tools Used

Manual analysis

Recommended Mitigation Steps

This said, it is unsafe to measure time with block.number and shall be done with block.timestamp, which is accepted by most nodes iff the real timestamp doesn't differ by more than 15 seconds.

loki-sama commented 2 years ago

We want the basket to be Locked for specific amount of blocks. So this common practice does not apply here.

0xleastwood commented 2 years ago

Agree with sponsor here. It makes sense to be block specific.