veRWA - pep7siup's results

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/main/src/LendingLedger.sol#L174

Vulnerability details

Impact

Claiming in a lump sum within the lending market can cause loyal lenders' income to be diluted at the last second of an epoch. This issue disincentivizes lenders from staying invested in the lending market for the entire epoch, or even longer. Malicious actors can exploit this vulnerability by entering the market at the last moment, claiming rewards disproportionately, and then swiftly moving their liquidity to another platform. This practice leads to the unfair distribution of rewards and undermines the attractiveness and fairness of the lending platform.

Proof of Concept

// Findings are labeled with '<= FOUND'
// File: src/LendingLedger.sol
152:    function claim(
153:        ...
174:        cantoToSend += (marketWeight * userBalance * ri.amount) / (1e18 * marketBalance); // <= FOUND: Lender's latest rewards can be diluted by late comer
179:        ...
182:    }

Line 174 shows that a userBalance recorded in the last second of the epoch is the same as one recorded from the epoch begin. This leads to the same cantoToSend amount to claim.

Below is the exploit scenario:

1. Alice, a lender, supplies 1e18 underlying tokens to the lending market.
2. The system takes a snapshot into the LendingLedger, recording Alice's userBalance and marketBalance as 1e18 each.
3. At timestamp currEpoch + WEEK - 1, Bob joins the market by depositing 10e18 tokens.
4. The system syncs the ledger for the current epoch, considering Alice's and Bob's balances and updating the userBalance and marketBalance.
5. After 1 second, a new epoch begins, and the rewards from the previous epoch are distributed.
6. Due to Bob's late entry, his rewards are disproportionately high, as his userBalance contribution of 10e18 tokens significantly impacts the rewards allocation.
7. Bob claims rewards worth 10e18/11e18 of the CANTO allocated for the market, diluting the rewards that loyal lenders like Alice should have received.

Tools Used

Manual Review

Recommended Mitigation Steps

Implement a more equitable rewards distribution mechanism. The Ledger should track the timestamp when a user joins the market, and the sync_ledger function should use this timestamp to gradually increase the eligible balance for rewards allocation. This increase should follow a slope of _delta / WEEK to ensure that latecomers' rewards are proportionate to their contributions and that loyal lenders' rewards are not unfairly diluted.

      _________               ________
     |                       /   
     |                      /     
_____|                ___  /      
     S,M       epoch       S  M     (M - S = 1 WEEK)

S: sync_ledger timestamp M: balance mature timestamp

Assessed type

MEV

Lines of code

https://github.com/code-423n4/2023-08-verwa/tree/main/src/VotingEscrow.sol#L383

Vulnerability details

Impact

Users, who delegated their lock and have their lock expired, are DOSed when attempting to withdraw their funds. The user is effectively locked out of their funds and any interaction with the contract is rendered impossible.

Proof of Concept

// File: src/VotingEscrow.sol
356:    function delegate(address _addr) external nonReentrant {
357:    ...
382:        require(toLocked.amount > 0, "Delegatee has no lock");
383:        require(toLocked.end > block.timestamp, "Delegatee lock expired"); // <= FOUND: user expired lock is DOS from withdrawal
384:        require(toLocked.end >= fromLocked.end, "Only delegate to longer lock");
385:        _delegate(delegatee, fromLocked, value, LockAction.UNDELEGATE);
386:        _delegate(_addr, toLocked, value, LockAction.DELEGATE);
387:    }

1. Alice decides to delegate her lock to Bob's address for a certain period.
2. After her lock expired, Alice attempts to withdraw her funds from the contract by un-delegating from Bob's lock first.
3. Alice's un-delegation call reverts at Line 383 due to expiration check against her lock.
4. Alice can no longer withdraw, create new locks, increase lock amounts, or even vote on proposals. This effectively locks Alice out of her funds and the contract permanently.

Below test confirms the finding:

// File: src/test/VotingEscrow.t.sol
function testUnDelegateExpired() public {
    // delegate from user1 to user2
    vm.prank(user1);
    ve.createLock{value: LOCK_AMT}(LOCK_AMT);
    vm.prank(user2);
    ve.createLock{value: LOCK_AMT}(LOCK_AMT);
    vm.prank(user1);
    ve.delegate(user2);
    (, uint256 end, , address delegatee) = ve.locked(user1);
    assertEq(delegatee, user2);
    (, , int128 delegated, ) = ve.locked(user2);
    assertEq(delegated, int128(int256(LOCK_AMT * 2)));

    // user1's lock expired
    vm.warp(end + 1);

    // undelegate to prepare for withdrawal, etc.
    vm.startPrank(user1);
    vm.expectRevert("Delegatee lock expired");
    ve.delegate(user1);

    // @audit since Undelegation is DOSed, all functions regarding lock management are rendered inoperable
    vm.expectRevert("Lock delegated");
    ve.withdraw();
    vm.expectRevert("Lock expired");
    ve.increaseAmount{value: 1}(1);
    vm.expectRevert("Lock exists");
    ve.createLock{value: 1}(1);
    vm.stopPrank();
}

Tools Used

Manual Review

Recommended Mitigation Steps

Allow lock owners to undelegate their locks even if the locks have already expired.

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/main/src/GaugeController.sol#L213 https://github.com/code-423n4/2023-08-verwa/blob/main/src/GaugeController.sol#L238-L242

Vulnerability details

Impact

A user who has voted on a gauge that gets removed becomes unable to revoke their votes. This vulnerability can lead to a loss of up to 100% of the user's votes. In addition, the user's voting power becomes tied to the removed gauge, preventing them from using their votes on other gauges effectively. This could result in the user's veRWA becoming less powerful or even entirely useless.

Proof of Concept

1. Alice participates in the reward distribution process by voting on Gauge A with 1,000 voting weights. Her vote_user_power is recorded as 1,000.
2. However, at some point, the governance decides to remove Gauge A, making it invalid for voting.
3. When Alice tries to call the vote_for_gauge_weights function with a weight of 0 to revoke her votes from the removed gauge, the transaction reverts at Line 213 which effectively denies a vote_user_power reset at Line 242.

// Findings are labeled with '<= FOUND'
// File: src/GaugeController.sol
211:    function vote_for_gauge_weights(address _gauge_addr, uint256 _user_weight) external {
212:        require(_user_weight >= 0 && _user_weight <= 10_000, "Invalid user weight");
213:        require(isValidGauge[_gauge_addr], "Invalid gauge address"); // <= FOUND: user cannot revoke their votes on removed gauge
212:        ...
238:        // Check and update powers (weights) used
239:        uint256 power_used = vote_user_power[msg.sender]; 
240:        power_used = power_used + new_slope.power - old_slope.power;
241:        require(power_used >= 0 && power_used <= 10_000, "Used too much power");
242:        vote_user_power[msg.sender] = power_used; // <= FOUND: cannot be reset
243:        ...
278:    }

This vulnerability leaves Alice's veRWA less influential or completely ineffective, as her voting power is tied to a non-existent gauge.

Tools Used

Manual Review

Recommended Mitigation Steps

To address this vulnerability, the contract should implement a revoke_votes_for_gauge function that allows users to reset their vote_user_power and vote_user_slopes for a gauge that has been removed. This function should be designed to work only when the user has previously voted on the specific gauge and when the gauge has been removed.

function revoke_votes_for_gauge(address _gauge_addr) external {
    require(!isValidGauge[_gauge_addr], "Gauge must be removed");
    require(vote_user_power[msg.sender] > 0, "No power used");
    
    uint256 stuck_power = vote_user_slopes[msg.sender][_gauge_addr].power;
    require(stuck_power > 0, "No power voted on this gauge");
    
    vote_user_power[msg.sender] -= stuck_power;
    delete vote_user_slopes[msg.sender][_gauge_addr];
}

Assessed type

Other