veRWA - ppetrov's results

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/498a3004d577c8c5d0c71bff99ea3a7907b5ec23/src/LendingLedger.sol#L117-L143

Vulnerability details

Impact

For a user to receive rewards for supplying cNote in a lending market (LM), he only needs to have supplied the cNote at the end of an epoch. Users staking for the whole duration of an epoch get 0 benefits, compared to users who supply only at the end of the epoch.

Therefore evRWA provides no incentive for users to supply cNote long-term in a specific LM.

Since there is no incentive to stick to one LM, this significantly reduces the incentive to lock cNote for evRWA - there is no need to vote for an LM because the user can choose the LM with the highest weight and redeem the rewards for it with 0 commitment.

This will create an unhealthy dynamic of reward-hunting without long-term commitment to the protocol.

Even if a LM provides good incentives for the user to keep tokens supplied, it is a reasonable assumption to make that protocols with even better incentives will exist.

I believe this to be a high issue because it can be detrimental to the success of the veRWA system. In order to prevent the issue from manifesting, the protocol will have to rely on the LMs to over-incentivize or to enforce long minimum deposit durations. No such requirements for the LMs have been described in the audit docs, which leads me to believe that no such requirements have been considered.

Proof of Concept

1. On deposit, LendingLedger::sync_ledger is called, which updates the deposited amount for the current epoch.

function sync_ledger(address _lender, int256 _delta) external { 
        address lendingMarket = msg.sender; 
        require(lendingMarketWhitelist[lendingMarket], "Market not whitelisted");

        _checkpoint_lender(lendingMarket, _lender, type(uint256).max);
        uint256 currEpoch = (block.timestamp / WEEK) * WEEK;
        int256 updatedLenderBalance = int256(lendingMarketBalances[lendingMarket][_lender][currEpoch]) + _delta;
        require(updatedLenderBalance >= 0, "Lender balance underflow"); // Sanity check performed here, but the market should ensure that this never happens

        lendingMarketBalances[lendingMarket][_lender][currEpoch] = uint256(updatedLenderBalance);

        _checkpoint_market(lendingMarket, type(uint256).max);
        int256 updatedMarketBalance = int256(lendingMarketTotalBalance[lendingMarket][currEpoch]) + _delta;
        require(updatedMarketBalance >= 0, "Market balance underflow"); // Sanity check performed here, but the market should ensure that this never happens
        lendingMarketTotalBalance[lendingMarket][currEpoch] = uint256(updatedMarketBalance);
    }

2. As soon as the epoch finishes, the LenderBalance becomes final. User can now withdraw everything from the Lending Market and keep any rewards.

Tools Used

Manual review

Recommended Mitigation Steps

To encourage supplying tokens over the whole duration of an epoch, changes to the reward calculation mechanism can be made. An alternative would be to enforce a rule for minimum LM deposit duration of 1 Epoch in order to be whitelisted. But this might not be viable due to the varying natures of the different lending markets.

Protocol changes could look like:

Instead of using the absolute balance of the user to calculate rewards, the protocol can use a linearly increasing average balance. This can be employed with simple math as in Example_1 or by using the already familiar slope of a line (increasing).

Example_1: epoch = 100 seconds Starting totalSupply = 1000 tokens

1. In the beginning Alice supplies 10 tokens.
2. After 95 seconds, Alice supplies 1000 more tokens. They are supplied for 5/100 seconds.

Alice average supply is 95/10010 + 5/1001010 = 9.5 + 50.5 = 60 tokens Average total supply = 101095/100 + 20105/100 = 959.5 + 100.5 = 1060 tokens

Alice's share = 60/1060 tokens = 5.66&

Example_2:

epoch = 100 seconds Starting totalSupply = 1000 tokens

1. Alice supplies 10 tokens. Her line starts with bias 0 and slope 10*(time_passed/epoch_duration).
2. After 95 seconds, alice supplies 1000 more tokens. Her line is recalculated from her previous line:

• Bias = 0 + 10*(95/100)
• slope = 1010 *(time_passed/epoch_duration)

3. At the end of the 100s her total tokens = 1095/100 + 10105/100 = 60 tokens (just like in Example_1)

Assessed type

Context

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/498a3004d577c8c5d0c71bff99ea3a7907b5ec23/src/LendingLedger.sol#L152

Vulnerability details

Impact

If rewards are accidentally not set for a single epoch, this will cause a revert in all transactions, trying to claim their rewards for period 0 -> type(uint256).max

The provided revert does not indicate for which epoch there is no rewards set and will force the user to either

1. Send many transactions for single epoch each, in order to not skip epochs with rewards.
2. Claim in smaller chunks and potentially skip some rewards.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Tools Used

Recommended Mitigation Steps

Assessed type

Error