c4-bot-5 commented 9 months ago

Lines of code

https://github.com/code-423n4/2023-12-autonolas/blob/2a095eb1f8359be349d23af67089795fb0be4ed1/governance/contracts/veOLAS.sol#L386-L389

Vulnerability details

Impact

tokens can be deposited and immediately withdrawn before the intended lock time by depositing right before expiry.

Proof of Concept

There is edge cases around the locking and unlocking periods that are not fully considered in the contract. Specifically, Deposits just before expiry:

The contract does not properly handle deposits made right before a lock expires

The code does not properly check or handle deposits made right before a lock expires. For example:

   if (lockedBalance.endTime < (block.timestamp + 1)) {
     revert LockExpired(msg.sender, lockedBalance.endTime, block.timestamp);
   }

This only checks if the lock has already expired. But if the lock is expiring in the next block, deposits could still be made. This means tokens may be deposited into an expired lock

For example:

Alice creates a 1 year lock for 100 tokens
After 364 days, Alice deposits another 50 tokens into the lock
The next day, the initial 1 year lock expires

In this case, the additional 50 token deposit is exposed to the vulnerability of withdrawing before the lock time. The contract should not allow additional deposits right before expiry to prevent this.

Tools Used

Manual

Recommended Mitigation Steps

Addi a check in the depositFor and increaseAmount functions to prevent deposits within a certain window (e.g. 1 week) before the lock expiry:

   function depositFor(address account, uint256 amount) external {
     LockedBalance memory lockedBalance = mapLockedBalances[account];

     // Only allow deposit if lock expires > 1 week from now
     require(lockedBalance.endTime > (block.timestamp + 1 weeks), "Deposit 
   too close

Assessed type

Other

c4-pre-sort commented 9 months ago

alex-ppg marked the issue as insufficient quality report

dmvt commented 8 months ago

feature, not bug

c4-judge commented 8 months ago

dmvt marked the issue as unsatisfactory: Invalid