code-423n4 / 2024-03-neobase-findings

0 stars 0 forks source link

Users will have the ability to vote for the gauge weights an infinite number of times, whenever the `VotingEscrow` withdrawals are unlocked #14

Closed c4-bot-10 closed 8 months ago

c4-bot-10 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-03-neobase/blob/d6e6127e6763b93c23ee95cdf7622fe950d9ed30/src/VotingEscrow.sol#L357

Vulnerability details

Impact

Users will be able to massively increase the gauge weights for particular markets, which in turn will give them the ability to claim much more tokens from the LendingLedger than they should be able to

Proof of Concept

Since the last audit of the codebase here at Code4rena, a new state variable named unlockOverride has been added to the VotingEscrow contract. By default, its value is set to false, but it can also be toggled to true through a governance proposal. Once it is set to true, it allows everyone to call the withdraw function and withdraw their locked CANTO immediately, no matter if their lock period has passed or not. This is due to the addition of the second condition in the following require statement:

require(locked_.end <= block.timestamp || unlockOverride, "Lock not expired");

Unfortunately though, this can easily be abused in order to game the gauge weight voting functionality of the GaugeController contract. Once the withdrawal of locked tokens is unlocked, malicious users can simply come in and perform the following actions:

  1. Create a lock through the VotingEscrow::createLock function;
  2. Call the GaugeController::vote_for_gauge_weights in order to make a vote;
  3. Call VotingEscrow::withdraw in order to withdraw their funds;
  4. Transfer their CANTO to another EoA controlled by them and repeat the first 3 steps

The above 4 steps can be repeated an infinite number of times, as long as unlockOverride is equal to true. What's worse is that in order for unlockOverride to be toggled back to false, a new governance proposal for doing so has to go through, meaning that the malicious users will have plenty of time to exploit that vulnerability before the protocol has a chance to react to it. And on a final note, if the malicious users have access to CANTO flash loans, then this means that they will also be able to exploit the voting system even further using them, since the above mentioned actions can be executed within a single transaction.

Tools Used

Manual Review

Recommended Mitigation Steps

Do not allow the creation of new locks when the withdrawals are unlocked.

Alternatively, you can also consider allowing users to only decrease their vote weights when the withdrawals are unlocked.

Assessed type

Other

c4-judge commented 8 months ago

MarioPoneder marked the issue as duplicate of #5

MarioPoneder commented 7 months ago

Moving forward with Medium severity since unlockOverride is under governance control which alleviates the issue.

c4-judge commented 7 months ago

MarioPoneder changed the severity to 2 (Med Risk)

c4-judge commented 7 months ago

MarioPoneder marked the issue as satisfactory