search

code-423n4 / 2022-05-aura-findings

0 stars 1 forks source link

Locking up AURA Token does not increase voting power of individual #186

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraLocker.sol#L258

Vulnerability details

Background

Per the documentation, AURA tokens can be locked in the AuraLocker to recieve vlAURA. vlAURA is voting power in the AURA ecosystem.

It is also possible for the users to delegate their voting power to a specific address by calling the AuraLocker.delegate(address account) function.

However, after users locked up their AURA tokens in exchange for vlAURA tokens, their voting power did not increase.

Proof of Concept

The following shows an example of Alice attempting to get some voting power by locking up her AURA tokens, but her voting power did not increase:

  1. At this point, Alice has not locked any AURA token into the AuraLocker yet. Thus, when AuraLocker.getVotes(Alice.address) is called, it returned “0” (No voting power. This is expected).

  2. Alice decided to get some voting power. So, Alice locked 100 AURA tokens by calling the AuraLocker._lock() function, and gain 100 vlAURA in return.

  3. Alice understand that as per the design, voting power will be 0 after depositing until the next epoch. So, she waited for around 1 week.

  4. After a week has passed, the AuraLocker.getVotes(Alice.address) is called again. Alice expected it to return"100", but it still returned “0” (Still no voting power).

  5. Alice has locked up her AURA tokens for a week and hold 100 vlAURA, yet she has no voting power.

The following snippet of test script demonstrates the above issue, showing that the vote power remains the same after locking up the AURA tokens for a week.

it("(Debug) allows users to lock aura", async () => {
    const cvxBalance = await phase4.cvx.balanceOf(stakerAddress);
    const lockBefore = await phase4.cvxLocker.lockedBalances(stakerAddress);
    console.log("(Debug) User Locked Balance Record = Total %s CVX (Unlockable = %s CVX, Locked = %s CVX) ",lockBefore.total, lockBefore.unlockable, lockBefore.locked);

    console.log("(Debug) User is going to lock %s CVX", cvxBalance)
    await phase4.cvx.connect(staker.signer).approve(phase4.cvxLocker.address, cvxBalance);
    await phase4.cvxLocker.connect(staker.signer).lock(stakerAddress, cvxBalance);

    const lockAfter = await phase4.cvxLocker.lockedBalances(stakerAddress);
    console.log("(Debug) User Locked Balance Record = Total %s CVX (Unlockable = %s CVX, Locked = %s CVX) ",lockAfter.total, lockAfter.unlockable, lockAfter.locked);

    expect(lockAfter.locked.sub(lockBefore.locked)).eq(cvxBalance);
});
it("(Debug) check user has votes after locking", async () => {
    const votesBefore = await phase4.cvxLocker.getVotes(stakerAddress);
    const lock = await phase4.cvxLocker.lockedBalances(stakerAddress);
    console.log("(Debug) votesBefore = %s, locked CVX = %s", votesBefore, lock.locked);
    console.log("(Debug) Properly locked tokens as of the most recent eligible epoch = %s", await phase4.cvxLocker.balanceOf(stakerAddress));

    await increaseTime(ONE_WEEK);
    console.log("After 1 week")

    const votesAfter = await phase4.cvxLocker.getVotes(stakerAddress);
    console.log("(Debug) votesAfter = %s, locked CVX = %s", votesBefore, lock.locked);
    console.log("(Debug) Properly locked tokens as of the most recent eligible epoch = %s", await phase4.cvxLocker.balanceOf(stakerAddress));

    expect(votesAfter.sub(votesBefore)).eq(lock.locked);

});
it("(Debug) check user lock balance and votes after 20 weeks", async () => {
    const TWENTY_WEEKS = BN.from(60 * 60 * 24 * 7 * 20);
    await increaseTime(TWENTY_WEEKS);
    console.log("(Debug) After 20 weeks")

    const lockAfter20 = await phase4.cvxLocker.lockedBalances(stakerAddress);
    console.log("(Debug) User Locked Balance = Total %s CVX (Unlockable = %s CVX, Locked = %s CVX) ",lockAfter20.total, lockAfter20.unlockable, lockAfter20.locked);
    console.log("(Debug) Properly locked tokens as of the most recent eligible epoch = %s", await phase4.cvxLocker.balanceOf(stakerAddress));

    expect(lockAfter20.unlockable).eq(lockAfter20.total); // all locks should have expired after 20 weeks.
});

Following is the output of the test script.

  1. The first section shows that user has 800563688188805506352 vlAURA after locking up their AURA tokens

  2. The second section shows that after a week, the user has 0 voting power even though the user has 800557536376417310407 vlAURA tokens. Note that these vlAURA tokens are all properly locked tokens that have not been expired.

(Note: vlAURA == vlCVX and AURA == CVX in this context)

        aura locker
(Debug) User Locked Balance Record = Total 0 CVX (Unlockable = 0 CVX, Locked = 0 CVX) 
(Debug) User is going to lock 800563688188805506352 CVX
(Debug) User Locked Balance Record = Total 800563688188805506352 CVX (Unlockable = 0 CVX, Locked = 800563688188805506352 CVX) 
          ✓ (Debug) allows users to lock aura

(Debug) votesBefore = 0, locked CVX = 800563688188805506352
(Debug) Properly locked tokens as of the most recent eligible epoch = 0
After 1 week
(Debug) votesAfter = 0, locked CVX = 800563688188805506352
(Debug) Properly locked tokens as of the most recent eligible epoch = 800563688188805506352
          1) (Debug) check user has votes after locking

(Debug) After 20 weeks
(Debug) User Locked Balance = Total 800563688188805506352 CVX (Unlockable = 800563688188805506352 CVX, Locked = 0 CVX) 
(Debug) Properly locked tokens as of the most recent eligible epoch = 0
          ✓ (Debug) check user lock balance and votes after 20 weeks

Aura Finance has implemented a checkpointing mechanism for determine user's voting power. Therefore, accounting for the votes will only happens during checkpoint when AuraLocker.checkpointDelegate() function is being called. Therefore, the AuraLocker.getVotes() function will only consider the locked AURA tokens that have been “checkpointed” as votes. In other words, if the locked AURA tokens have not been “checkpointed” yet, it will simply remain as a balance in the AuraLocker contract, and the user’s locked AURA tokens effectively have no voting power.

Based on the source code, the root cause of this issue is that if a user does not have a delegatee, the system will not perform any checkpointing, and user's locked AURA token will not be accounted as voting power.

Following code from AuraLocker._lock() shows that checkpointing will only be performed if the user has a delegatee. Otherwise, no checkpointing will be performed when users locked their AURA tokens.

function _lock(address _account, uint256 _amount) internal {
    ..SNIP..
    address delegatee = delegates(_account);
    if (delegatee != address(0)) {
        delegateeUnlocks[delegatee][unlockTime] += lockAmount;
        _checkpointDelegate(delegatee, lockAmount, 0);
    }
    // @audit - No checkpointing performed for the rest of the code in this function
    ..SNIP..
}

The only way for Alice could get back her voting power is to delegate to herself after locking her AURA tokens. This is a workaround. AuraLocker.delegate() sole purpose should only serve to delegate one’s voting power to another user, and should not be used as a workaround to force the system to perform checkpointing to gain voting power.

For Alice to get back her voting power, she must call the AuraLocker.delegate(Alice.address) function, which will delegate to herself. This function will in turn call the AuraLocker._checkpointDelegate() function, which will “checkpointed” Alice’s locked tokens to become votes. Only after this step, Alice’s voting power will be updated and calling AuraLocker.getVotes(Alice.address) should return “100” now.

Additionally, documentation did not mention that a user is required to delegate to oneself in order to get the voting power. Thus, it is very likely that majority of the users would not know how to get their voting power unless they review the source code or is aware of this workaround.

Impact

The impact of this issue is that users might miss the opportunity to vote on critical protocol decisions or flow of incentives (Gauge voting) due to lack of voting power as voting power is not assigned to them after locking up AURA tokens.

If the users only realised this issue in the current epoch, they would miss the chance to vote in current epoch. This is because by calling the AuraLocker.delegate(address account) function to fix the issue, the votes will only be effective in the next epoch.

The outcome of the governance or gauge voting might be impacted and might not reflect the true consensus of the community as affected users are not able to participate in the vote or have inaccurate voting power, thus affecting the protocol.

Tools Used

Manual Inspection

Recommended Mitigation Steps

In Convex Finance, users lock their CVX tokens by calling CvxLocker._lock() function and voting power will be allocated to the users immediately. Similar strategy should be adopted.

It is recommended to update the AuraLocker._lock() function so that the user’s locked AURA tokens are “checkpointed” and converted to voting power immediately after locking up if a user has not assigned a delegatee yet. This will trigger the accounting for votes and translate the newly locked tokens into voting power immediately.

Original Code

function _lock(address _account, uint256 _amount) internal {
    ..SNIP..
    address delegatee = delegates(_account);
    if (delegatee != address(0)) {
        delegateeUnlocks[delegatee][unlockTime] += lockAmount;
        _checkpointDelegate(delegatee, lockAmount, 0);
    }
    ..SNIP..
}

Suggested Modification

function _lock(address _account, uint256 _amount) internal {
    ..SNIP..
    address delegatee = delegates(_account);
    if (delegatee != address(0)) {
        delegateeUnlocks[delegatee][unlockTime] += lockAmount;
        _checkpointDelegate(delegatee, lockAmount, 0);
    } else {
        // If there is no delegatee, 
        // then automatically delegate to the account to trigger the checkpointing
        delegateeUnlocks[_account][unlockTime] += lockAmount;
        _checkpointDelegate(_account, lockAmount, 0);
    }
    ..SNIP..
}
0xMaharishi commented 2 years ago

Users must simply delegate to themselves to receive voting power

dmvt commented 2 years ago

Valid issue. Fix the documentation or the code. If all users need to do is delegate to themselves, then auto-delegating newly minted votes to the user would solve the issue.