Core Contracts

Vulnerability Details

A user can reset their decay in voting power by increasing the amount lock on veToken.

This should not be the behaviour because:
- Someone can lock 1_000 tokens for 4 years. (veToken::lock(1_000, 4 years))
- At the second year there is a new proposal. The user has a decreased voting power due to decay.
- He increases that lock by 1 token. The voting power is re-calculated and the decay is reset. (veToken::increase(1))
- This effectively gives this user staking 1 token the same voting power as staking 1_000 tokens for 4 years.
- Now the user can vote with unexpected power on the proposal.

Proof Of Concept

We can see that the above flow is true by seeing the following detailed links:

• This is how the user power is calculated in Governance, using VotingPowerLib::getCurrentPower().

  • Governance::castVote() -> veToken::getVotingPower() -> VotingPowerLib::getCurrentPower(). See first call here, second one here, which eventually calls the library here.

Now let's see the logic on the library used by veToken::increase().

Note: The library is out-of-scope, yet the fix can be done leveraging other functions in the library from the veTokencontract, which is in-scope.

The functions call goes like this: veToken::increase() -> VotingPowerLib::calculateAndUpdatePower(). See the call here.

Now inside this function the user's Point timestamp is updated to the current timestamp. See here. This Point is crucial as it is a point on a linearly decaying function that represents the user's voting power. And the updated timestamp is what it is used to later deduce time passed and calculate the decay on VotingPowerLib::getCurrentPower(), the one called by governance.

See how the timestamp from the point determines the decay here. This is the timeDelta calculation that a few lines below multiplies the slope to get the decay, here. So just after increase, the timeDelta will be 0, and the decay will be 0, effectively resetting the decay.

So, summping up, user stakes 4 years X amount, time passes, new governance proposal, user calls increase with just 1 token and resets the decay due to the update of the timestamp of his Point. Mix this increase call with a following castVote() in the same tx and the user will have unexpected voting power on the proposal.

Impact

This vulnerability allows a user to reset their decay in voting power by increasing the amount lock on veToken. This can lead to unexpected voting power on proposals.

It also creates an unfair advantage to users that know this issue, as someone who fairly stakes for 4 years 1_000 tokens will have less voting power than someone who just staked 1 token more and leveraged this vulnerability.

Recommendations

Do not use calculateAndUpdatePower() when veToken::increase() as this updates the user's Point timestamp. You should instead:

This way timestamp will still be reset, yet the amount of tokens distributed in the remaining time will be the actual amount the user already had active taking into account the decay so far.

This is because voting power is the same as active token amounts, having the same units as token amounts.