Potential Loss of Funds in WiseLending Contract Due to Incorrect Repayment Calculations

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L1161-L1198 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L1204-L1238

Vulnerability details

Impact

Borrowers could lose the excess amount of ERC20 to be paid back, as it would be stuck inside the WiseLending contract and never refunded to the borrower.

Proof of Concept

This issue involves two scenarios where the paybackExactAmount() and paybackExactShares() functions are called with parameters that could lead to incorrect repayment calculations. The contract should include checks to ensure that the repayment amount and the corresponding shares do not exceed the borrower's current borrow shares.

Scenario 1: paybackExactAmount() Function https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L1161-L1198

Bob has a position in the WiseLending contract with an NFT ID of 12345. He has borrowed a token with the address 0xBorrowedToken and has a maximum of 200 borrow shares (maxBorrowShares) for this token. Bob calls the paybackExactAmount() function with an _amount of 1000, which results in 500 shares to repay. However, the contract does not include a check to ensure that paybackShares does not exceed maxBorrowShares, leading to an incorrect update of the contract's state and the loss of 800 units of the borrowed token.

Scenario 2: paybackExactShares() Function https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L1204-L1238

Alice has a position in the WiseLending contract with an NFT ID of 67890. She has borrowed a token with the address 0xBorrowedToken and has a maximum of 300 borrow shares (maxBorrowShares) for this token. Alice calls the paybackExactShares() function with _shares of 400, which results in 2000 units of the borrowed token to repay. However, the contract does not include a check to ensure that repaymentAmount does not exceed maxBorrowShares, leading to an incorrect update of the contract's state and the loss of 1700 units of the borrowed token.

Detailed difference between both scenarios:

Both issues involve incorrect repayment calculations that could lead to a loss of funds for borrowers. The first issue is related to the amount of the borrowed token being repaid, while the second issue is related to the number of shares being repaid. In both cases, the contract should include checks to ensure that the repayment amount and the corresponding shares do not exceed the borrower's current borrow shares.

Tools Used

Manual Review

Recommended Mitigation Steps

Include a check in the _handlePayback() function to ensure that the paybackShares do not exceed the maxBorrowShares when the paybackExactAmount() function is called. Also Include a check in the _handlePayback() function to ensure that the repaymentAmount does not exceed the maxBorrowShares when the paybackExactShares() function is called.

Assessed type

ERC20

[GalloDaSballo]

Seems incorrect, since the value is bound

https://github.com/code-423n4/2024-02-wise-lending/blob/1240a22a3bbffc13d5f8ae6300ef45de5edc7c19/contracts/InvariantUser.sol#L593-L598

        _shares = bound(
            _shares,
            0,
            lendingUserInstance.getPositionBorrowShares(_nftId, tokens[_tokenIndex])
        );

[c4-pre-sort]

GalloDaSballo marked the issue as insufficient quality report

[c4-pre-sort]

GalloDaSballo marked the issue as primary issue

[vm06007]

Should be marked as invalid.

[c4-judge]

trust1995 marked the issue as unsatisfactory: Invalid