veToken Finance contest - Chom's results

Lines of code

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeAssetDepositor.sol#L102-L109

Vulnerability details

Impact

uint256 unlockInWeeks = (unlockAt / WEEK) * WEEK; is in seconds (Rounded by seconds in 1 week) instead of weeks

Causing unlockTime logic to be broken

For example unlockInWeeks.sub(unlockTime) is 7 * 86400 when the time has passed by 1 week instead of 1

Proof of Concept

uint256 unlockInWeeks = (unlockAt / WEEK) * WEEK;

unlockAt is in seconds WEEK is in seconds / week (seconds in 1 week)

(unlockAt / WEEK) * WEEK has unit (seconds / (seconds/week)) * (seconds/week) = seconds

Can be proven by this example

Epoch timestamp: 1654189731 Date and time (GMT): Thursday, June 2, 2022 17:08:51 (unlockAt / WEEK) * WEEK = 1654128000 store this value to unlockTime

Pass time by 7 days = 1 week

Epoch timestamp: 1654794531 Date and time (GMT): Thursday, June 9, 2022 17:08:51 (unlockAt / WEEK) * WEEK = 1654732800

unlockInWeeks.sub(unlockTime) = 1654732800 - 1654128000 = 604800 = 7 * 86400

But we expect unlockInWeeks.sub(unlockTime) to be 1 as time is passed by 1 week Instead we got 7 * 86400 Which is wrong

Tools Used

• Scan code by eye
• Python CLI to proof of concept

Recommended Mitigation Steps

uint256 unlockInWeeks = (unlockAt / WEEK);

Please don't multiply by WEEK again