code-423n4 / 2024-08-wildcat-findings

3 stars 1 forks source link

lenders are unable to get full repayment amount #34

Closed howlbot-integration[bot] closed 1 month ago

howlbot-integration[bot] commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/market/WildcatMarketWithdrawals.sol#L251 https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/market/WildcatMarketWithdrawals.sol#L247

Vulnerability details

The executeWithdrawal function does not account for delinquent debt payment. It calculates the withdrawal amount based on the batch's normalizedAmountPaid and the user's scaledAmount, but it doesn't factor in any additional funds that may have been repaid during a delinquency period leading to loss of funds for lenders

Proof of Concept

https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/market/WildcatMarketWithdrawals.sol#L212

https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/market/WildcatMarketWithdrawals.sol#L247

https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/market/WildcatMarketWithdrawals.sol#L251

The issue is as follows :

  1. Lender deposits 100 units into market.
  2. Borrower borrows 70 units.
  3. Lender queues full withdrawals.
  4. Borrower repays .
  5. Lender only receives 30 units .

Paste this in WildcatMarketWithdrawalsTest.sol::WithdrawalsTest

function test_Poc() external {
    // Create a lender address
    address lender = vm.addr(0x123);

    // Authorize the lender 
    _authorizeLender(lender);

    // Lender deposits 100 units into the market
    _deposit(lender, 100);

    // Verify the lender's balance 
    assertEq(market.balanceOf(lender), 100);
    assertEq(asset.balanceOf(address(market)), 100);

    // Borrower borrows 70 units from the market
    vm.startPrank(borrower);
    market.borrow(70);
    vm.stopPrank();

    fastForward(parameters.withdrawalBatchDuration + 1);
    market.updateState();

    // Lender requests a full withdrawal of their funds
    vm.prank(lender);
    market.queueFullWithdrawal();
    uint256 expiry = block.timestamp + parameters.withdrawalBatchDuration;

    fastForward(20 days);
    market.updateState();

    // Borrower repay delinquent debt
    vm.startPrank(borrower);
    asset.mint(borrower, 100);
    asset.approve(address(market), 100);

    // Verify borrower is  delinquent 
    MarketState memory state0 = market.previousState();
    assertEq(state0.isDelinquent, true);

    // Borrower repays the delinquent debt
    market.repayDelinquentDebt();

    asset.mint(borrower, 100);
    asset.approve(address(market), 100);

    // Execute the withdrawal for the lender
    market.executeWithdrawal(lender, uint32(expiry));
    vm.stopPrank();

    market.updateState();
    MarketState memory state = market.previousState();

// borrower is no longer delinquent
    assertEq(state.isDelinquent, false);

    // Check the lender's final balance (should be 0 in the market)
    uint256 finalBalance = asset.balanceOf(lender);
    assertEq(market.balanceOf(lender), 0);    

    // Borrower repays the remaining debt
    vm.startPrank(borrower);
    asset.mint(borrower, 100);
    asset.approve(address(market), 100);
    market.repay(70);
    vm.stopPrank();

   //@audit-issue : did not receive full amount 
    // The lender only receives 30 units instead of their initial 100
    assertEq(asset.balanceOf(lender), 30);
}

Recommended Mitigation Steps

Modify executeWithdrawal function to account for delinquent debt repayments

Assessed type

Other

laurenceday commented 1 month ago

This functionality is precisely covered by repayAndProcessUnpaidWithdrawalBatches, and the UI invokes either this or repay appropriately depending on whether or not the withdrawal queue is non-empty.

See: https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/market/WildcatMarketWithdrawals.sol#L283-L317

Not a finding.

3docSec commented 1 month ago

As per sponsor's comment, this finding highlights a failure happening in the scenario of a user error - the wrong function is called.

c4-judge commented 1 month ago

3docSec marked the issue as unsatisfactory: Invalid