code-423n4 / 2023-08-verwa-findings

8 stars 7 forks source link

First Time Claiming of Reward made impossible. #351

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/9a2e7be003bc1a77b3b87db31f3d5a1bcb48ed32/src/LendingLedger.sol#L158 https://github.com/code-423n4/2023-08-verwa/blob/9a2e7be003bc1a77b3b87db31f3d5a1bcb48ed32/src/LendingLedger.sol#L25-L26 https://github.com/code-423n4/2023-08-verwa/blob/9a2e7be003bc1a77b3b87db31f3d5a1bcb48ed32/src/LendingLedger.sol#L176 https://github.com/code-423n4/2023-08-verwa/blob/9a2e7be003bc1a77b3b87db31f3d5a1bcb48ed32/src/LendingLedger.sol#L159

Vulnerability details

Impact

First Time Claiming of Reward made impossible for Lenders.

Proof of Concept

When userClaimedEpoch mapping was created, it is meant to hold the last epoch time a lender claimed Reward.

  /// @dev Lending Market => Lender => Epoch
    mapping(address => mapping(address => uint256)) public userClaimedEpoch; // Until which epoch a user has claimed for a particular market (exclusive this value)

This value is usually set/updated after the Lender has claimed reward till a particular epoch, userClaimedEpoch[_market][lender] = claimEnd + WEEK;

However, before a Lender is able to claim Reward, it was required that userClaimedEpoch be greater than zero, require(userLastClaimed > 0, "No deposits for this user"); a condition that can NEVER be fulfilled for first time reward claimer(that is when claiming Reward for the first time) because the initial value will always be zero. Also, the error for that statement states "No deposits for this user", is a false statement, because it does not indicate the balance of the lender, rather, it indicates when last the lender claimed reward.

Tools Used

Manual review.

Recommended Mitigation Steps

mapping(address => mapping(address => mapping(uint256 => uint256))) public lendingMarketBalances; // cNote balances of users within the lending markets, indexed by epoch This is the variable that indicates the balance of the Lender in a particular lending market and epoch. The statement should be replaced with: require(lendingMarketBalances[_market][lender][epoch] > 0, "No deposits for this user");

Assessed type

Context

141345 commented 1 year ago

seems invalid

_checkpoint_lender() will make it > 0

c4-pre-sort commented 1 year ago

141345 marked the issue as low quality report

c4-pre-sort commented 1 year ago

141345 marked the issue as primary issue

alcueca commented 1 year ago

After a user makes a deposit, checkpoint_lender can be called which will initialize userClaimedEpoch. The require message is partially right in that for userClaimedEpoch to be zero either the user hasn't made any deposit, or checkpoint_lender wasn't called.

alcueca commented 1 year ago

Marking it as grade-b QA for the sponsor to take a look at the natspec and documentation.

c4-judge commented 1 year ago

alcueca changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

alcueca marked the issue as grade-b

c4-judge commented 1 year ago

This previously downgraded issue has been upgraded by alcueca

c4-judge commented 1 year ago

alcueca changed the severity to QA (Quality Assurance)