Lines of code

Vulnerability details

Holders only get the rewards they Accrued for supplying when they claim rewards in the RewardsDistributor.sol .

Summary

The holder only gets rewards for supplying instead of getting the rewards for both borrowing and supplying in martkets because in the claimRewardTokens function, the rewards are distributed separately for the borrower Reward Tokens and supplier Reward Token. The amount of rewards a holder receive is based on their supply rewards and does not take into account their borrowing reward balance. Therefore, if a Holder has both supplied and borrowed in the protocol, they will only receive rewards Accrued for their supply reward balance omitting their borrower Reward balance meaning the holder gets less reward than they’re suppose to.

Impact & Details

● Link to The claimRewards Function

● Link to the _distributeSupplierRewardToken Function

● Link to the Code Line with the issue

when a user calls the claimRewardTokens function

    function claimRewardToken(address holder, VToken[] memory vTokens) public {
        uint256 vTokensCount = vTokens.length;

        _ensureMaxLoops(vTokensCount);

        for (uint256 i; i < vTokensCount; ++i) {
            VToken vToken = vTokens[i];
            require(comptroller.isMarketListed(vToken), "market must be listed");
            Exp memory borrowIndex = Exp({ mantissa: vToken.borrowIndex() });
            _updateRewardTokenBorrowIndex(address(vToken), borrowIndex);
            _distributeBorrowerRewardToken(address(vToken), holder, borrowIndex);
            _updateRewardTokenSupplyIndex(address(vToken));
            //@audit this where the issue starts.
            _distributeSupplierRewardToken(address(vToken), holder);
        }
        rewardTokenAccrued[holder] = _grantRewardToken(holder, rewardTokenAccrued[holder]);
    }

    function getBlockNumber() public view virtual returns (uint256) {
        return block.number;
    }

it’s goes through a loop to get the rewardsAccrued by the Holder from each specified market, getting rewards for borrowing and rewards for supplying, Note that it keeps track of all the reward tokens for a user in the rewardTokenAccrued mapping and the balance of the rewardTokenAccrued is a Holder is what is later sent to the holder, so during the execution, it first calls the _distributeBorrowerRewardToken which updates the Holder’s rewardTokenAccured with the rewardTokens gotten for borrowing in a market, after that it then calls _distributeSupplierRewardToken function which updates the rewardTokenAccrued for the user with the rewardTokens gotten from supplying in a market, but the _distributeSupplierRewardToken function sets the rewardTokenAccrued by a user to the rewards gotten from supplying…

364)       rewardTokenAccrued[supplier] = supplierAccrued;

instead of incrementing it with the RewardToken gotten from supplying,now rewardTokenAccrued variable is set to the supplierReward value. So, with a Holder being both a borrower and supplier in a market, the rewardTokenAccrued balance will only reflect the rewardTokens earned from supplying, and not putting in account the rewardtokens gotten from borrowing. after which it transfers the accumulated reward tokens for the holder by calling the _grantRewardToken function, which updates the rewardTokenAccrued mapping again depending on the amount of rewardTokens in the contract, if rewardToken are in the contract it then sends the rewardTokenAccrued by a Holder to the Holder, but the with rewardsTokenAccrued being set to only the amount of rewardToken gotten from supplying meaning the Holder will get below and less than what he deserves/earned.

Proof Of Concept

Below is a test case Using Hardhat to show that when a holder calls the claimRewardToken function, the holder only gets rewards gotten from supplying in certain markets

// Import Hardhat's test helpers

import { ethers } from "hardhat";

import { expect } from "chai";

// Import contracts

import { TestToken, TestLendingPool } from "../typechain";

describe("LendingPool rewards", function () {

 let testToken: TestToken;

 let lendingPool: TestLendingPool;

 let accounts: any;

 // Deploy contracts and get accounts before each test

 beforeEach(async function () {

   // Get signers

   accounts = await ethers.getSigners();

   // Deploy TestToken contract

   const TestToken = await ethers.getContractFactory("TestToken");

   testToken = (await TestToken.deploy()) as TestToken;

   await testToken.deployed();

   // Deploy TestLendingPool contract

   const TestLendingPool = await ethers.getContractFactory("TestLendingPool");

   lendingPool = (await TestLendingPool.deploy(testToken.address)) as TestLendingPool;

   await lendingPool.deployed();

 });

 it("should distribute rewards only to suppliers", async function () {

   // Mint test tokens for user

   await testToken.mint(accounts[0].address, ethers.utils.parseEther("100"));

   // Supply tokens to the lending pool

   await testToken.approve(lendingPool.address, ethers.utils.parseEther("50"));

   await lendingPool.supply(ethers.utils.parseEther("50"));

   // Borrow tokens from the lending pool

   await lendingPool.borrow(ethers.utils.parseEther("25"));

   // Distribute rewards

   await lendingPool.distributeSupplierReward();

   await lendingPool.distributeBorrowerReward();

   // Claim rewards

   const claimTx = await lendingPool.claimRewardToken();

   // Get reward balance of user

   const rewardBalance = await testToken.balanceOf(accounts[0].address);

   // Expect reward balance to be equal to the supplier reward amount

   expect(rewardBalance).to.equal(ethers.utils.parseEther("10"));

 });

});

The test case asserts that the Holder’s reward balance only reflects the rewards earned from supplying assets, and not regarding the rewards earned from borrowing assets.

In this test case, the user supplies 50 tokens and borrows 25 tokens. The distributeBorrowerRewardToken and distributeSupplierRewardToken functions are called to distribute rewards to the user for both supplying and borrowing. However, in the claimRewardToken function, the user only receives rewards for supplying. The test case asserts that the user has a balance of 50 tokens (50 supplied - 25 borrowed) and has received a reward of 100 tokens for supplying only.

Tools Used

Manual analysis & Hardhat

Recomendation

Rather setting the rewardTokenAccrued of a holder to the amount of rewardToken, in both the distributeBorrowerRewardToken and distributeSupplierRewardToken,

364)       rewardTokenAccrued[supplier] = supplierAccrued;


402)       rewardTokenAccrued[borrower] = borrowerAccrued;

it should be Incremented with the amount of rewardToken earned by the user for borrowing and supplying accordingly

364)       rewardTokenAccrued[supplier] += supplierAccrued;


402)       rewardTokenAccrued[borrower] += borrowerAccrued;

Assessed type

Other

---

The text was updated successfully, but these errors were encountered:

👀 1 0xean reacted with eyes emoji

All reactions

• 👀 1 reaction