code-423n4 / 2022-08-fiatdao-findings

2 stars 1 forks source link

[H1] LockedBalance.delegated set to zero before checkpoint #315

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L540-L544 https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L646-L650

Vulnerability details

Impact

​ Checkpoint called with incorrect values

Proof of Concept

​ At quitLock() and withdraws() you set LockedBalance.delegated to zero. I is not explained in the documentation but as I understand this variable stores the amount of tokens delegated to this address by others accounts. Am I wrong? Setting to zero and then calling _checkpoint with this value will produce incorrect values if the account has been delegated votes from others.

Recommended Mitigation Steps

​ Remove

newLocked.delegated = 0;

lacoop6tu commented 2 years ago

this is an expected behaviour

elnilz commented 2 years ago

as @lacoop6tu indicated, this is not a bug but intended behavior. only users with an active lock maintain a non-zero virtual balance (i.e. voting power) in the system

gititGoro commented 2 years ago

Expected behaviour. Marking invalid.