Skip to content
This repository was archived by the owner on Oct 22, 2023. It is now read-only.
This repository was archived by the owner on Oct 22, 2023. It is now read-only.

0xbepresent - Malicious borrower can block liquidations causing the lender to receive neither the settlement amount nor the collateral #228

Description

@sherlock-admin

0xbepresent

high

Malicious borrower can block liquidations causing the lender to receive neither the settlement amount nor the collateral

Summary

Once the loan has started the borrower can modify the _bidCollaterals using the commitCollateral() function which causes that if the loan goes into default, the loan can not be liquidated and the lender can not receive neither the settlement amount nor the collateral.

Vulnerability Detail

Once the loan has started the commitCollateral() is available to be called by anyone, allowing to modify the _bidCollaterals. Then, if the loan goes into default, the _withdraw() will be reverted by "Withdraw amount cannot be zero" error.

Please see the next attack scenario:

  1. The loan has started.
  2. The loan is defaulted and the malicious borrower calls commitCollateral() and put the collateral amount to zero.
  3. The liquidator wants to liquidate via liquidateLoanFull() but is not possible because the function is reverted by "Withdraw amount cannot be zero" error.
  4. The lender wants to grab the collateral via withdraw() functions but is still not possible because the funcstion is reverted by "Withdraw amount cannot be zero" error.

Impact

The malicious borrower can block the liquidations and the lender collateral withdrawals causing the lender to receive neither the settlement amount nor the collateral. Additionally the borrower can gain time to repay his loan.

I created a test where the malicious borrower deny the liquidations and the collateral withdrawals. Test steps:

  1. Create the collateral bid and the lender accept it
  2. Assert the escrow has the borrower's collateral
  3. Malicious borrower modifies to zero the collateral amount via the commitCollateral() function
  4. Skip the time so the loan is defaulted
  5. The liquidations are reverted by "Withdraw amount cannot be zero" error
  6. The collateral withdrawals are reverted by "Withdraw amount cannot be zero" error
File: TellerV2_Test.sol
451:     function test_MaliciousBorrowerCanBlockLiquidations() public {
452:         // Malicious borrower can block liquidations causing the lender
453:         // to receive neither the settlement amount nor the collateral
454:         // 1. Create the collateral bid and lender accept it
455:         // 2. Assert the escrow has the borrower's collateral
456:         // 3. Malicious borrower modifies to zero the collateral amount
457:         // 4. Skip the time so the loan is defaulted
458:         // 5. The liquidations are reverted by "Withdraw amount cannot be zero" error
459:         // 6. The collateral withdrawals are reverted by "Withdraw amount cannot be zero" error
460:         //
461:         //
462:         // 1. Create the collateral bid and lender accept it
463:         //
464:         // Submit bid as borrower
465:         uint256 bidId = submitCollateralBid();
466:         // Accept bid as lender
467:         acceptBid(bidId);
468:         //
469:         // 2. Assert the escrow has the borrower's collateral
470:         //
471:         // Get newly created escrow and test the creates escrow has the same bidId and collateral stored
472:         address escrowAddress = collateralManager._escrows(bidId);
473:         CollateralEscrowV1 escrow = CollateralEscrowV1(escrowAddress);
474:         uint256 storedBidId = escrow.getBid();
475:         assertEq(bidId, storedBidId, "Collateral escrow was not created");
476:         uint256 escrowBalance = wethMock.balanceOf(escrowAddress);
477:         assertEq(collateralAmount, escrowBalance, "Collateral was not stored");
478:         //
479:         // 3. Malicious borrower modifies to zero the collateral amount.
480:         //
481:         Collateral memory maliciousInfo;
482:         maliciousInfo._amount = 0; // <-- zero value
483:         maliciousInfo._tokenId = 0;
484:         maliciousInfo._collateralType = CollateralType.ERC20;
485:         maliciousInfo._collateralAddress = address(wethMock);
486:         collateralManager.commitCollateral(bidId, maliciousInfo);
487:         //
488:         // 4. Skip the time so the loan is defaulted
489:         //
490:         Payment memory amountOwed = tellerV2.calculateAmountOwed(bidId);
491:         vm.warp(30 days);
492:         //
493:         // 5. The liquidations are reverted by "Withdraw amount cannot be zero" error
494:         //
495:         IERC20(address(daiMock)).approve(address(tellerV2), amountOwed.principal + amountOwed.interest);
496:         vm.expectRevert(bytes("Withdraw amount cannot be zero"));
497:         tellerV2.liquidateLoanFull(bidId);
498:         //
499:         // 6. The collateral withdrawals are reverted by "Withdraw amount cannot be zero" error
500:         //
501:         vm.expectRevert(bytes("Withdraw amount cannot be zero"));
502:         collateralManager.withdraw(bidId);
503:     }

Code Snippet

The commitCollateral() allows anyone to modify the collateral available.

File: CollateralManager.sol
117:     function commitCollateral(
118:         uint256 _bidId,
119:         Collateral[] calldata _collateralInfo
120:     ) public returns (bool validation_) {
121:         address borrower = tellerV2.getLoanBorrower(_bidId);
122:         (validation_, ) = checkBalances(borrower, _collateralInfo);
123: 
124:         if (validation_) {
125:             for (uint256 i; i < _collateralInfo.length; i++) {
126:                 Collateral memory info = _collateralInfo[i];
127:                 _commitCollateral(_bidId, info);
128:             }
129:         }
130:     }

In the attack scenario, the collateral is set to zero causing the liquidations or collateral withdrawals transactions will be reverted by amount zero error.

File: CollateralEscrowV1.sol
84:     function withdraw(
85:         address _collateralAddress,
86:         uint256 _amount,
87:         address _recipient
88:     ) external virtual onlyOwner {
89:         require(_amount > 0, "Withdraw amount cannot be zero");
90:         Collateral storage collateral = collateralBalances[_collateralAddress];

Tool used

Manual review

Recommendation

Don't allow collateral modifications once the loan has started.

Duplicate of #168

Activity

  1. added
    MediumA valid Medium severity issue
    DuplicateA valid issue that is a duplicate of an issue with `Has Duplicates` label
    on May 1, 2023
  2. added
    Non-RewardThis issue will not receive a payout
    and removed
    MediumA valid Medium severity issue
    DuplicateA valid issue that is a duplicate of an issue with `Has Duplicates` label
    on May 20, 2023
  3. 0xbepresent commented on May 20, 2023

    @0xbepresent

    Escalate for 15 USDC

    I consider my issue is still valid because my test demonstrates that the once the loan is defaulted, the borrower can not be liquidated. The liquidations are reverted by "Withdraw amount cannot be zero" error.

    Additionally the issue #170 mentions that the commitment needs to be accepted by the lender. Quote: TellerV2#lenderAcceptBid only allows the lender input the bidId of the bid they wish to accept, not allowing them to specify the expected collateral. This allows lenders to be honeypot and front-run causing massive loss of funds:

    The issue that I reported impacts the loans that are in process or loans that need to be liquidated. Totally different from the issue #170 where the lender needs to accept the loan then the malicious actor to front-run it.

    Since the issue I reported can make the lender to lost their loan and the borrow‘s collateral (unable to liquidate) I still consider my issue as a High.

    Thanks for your time and patience.

  4. sherlock-admin commented on May 20, 2023

    @sherlock-admin
    ContributorAuthor

    Escalate for 15 USDC

    I consider my issue is still valid because my test demonstrates that the once the loan is defaulted, the borrower can not be liquidated. The liquidations are reverted by "Withdraw amount cannot be zero" error.

    Additionally the issue #170 mentions that the commitment needs to be accepted by the lender. Quote: TellerV2#lenderAcceptBid only allows the lender input the bidId of the bid they wish to accept, not allowing them to specify the expected collateral. This allows lenders to be honeypot and front-run causing massive loss of funds:

    The issue that I reported impacts the loans that are in process or loans that need to be liquidated. Totally different from the issue #170 where the lender needs to accept the loan then the malicious actor to front-run it.

    Since the issue I reported can make the lender to lost their loan and the borrow‘s collateral (unable to liquidate) I still consider my issue as a High.

    Thanks for your time and patience.

    You've created an escalation for 15 USDC, however, the escalation amount is fixed at 10 USDC.
    To update the escalation with the correct amount, please edit this comment. (do not create a new comment)

    To remove the escalation from consideration: Delete your comment.

    You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

  5. Trumpero commented on Jun 8, 2023

    @Trumpero
    Collaborator

    I agree with the escalation. The second commitCollateral call in the scenario occurs after the bid is accepted. Therefore, this issue is separate from issue #170, and I believe it is a duplicate of issue #168 (commitCollateral can be called on an active loan).

  6. added
    RewardA payout will be made for this issue
    and removed
    Non-RewardThis issue will not receive a payout
    on Jun 8, 2023
  7. hrishibhat commented on Jun 8, 2023

    @hrishibhat

    Escalation accepted

    Valid duplicate of #168

  8. added
    Escalation ResolvedThis issue's escalations have been approved/rejected
    and removed
    EscalatedThis issue contains a pending escalation
    on Jun 8, 2023
  9. sherlock-admin commented on Jun 8, 2023

    @sherlock-admin
    ContributorAuthor

    Escalation accepted

    Valid duplicate of #168

    This issue's escalations have been accepted!
    
    Contestants' payouts and scores will be updated according to the changes made on this issue.
    
  10. added
    DuplicateA valid issue that is a duplicate of an issue with `Has Duplicates` label
    on Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    DuplicateA valid issue that is a duplicate of an issue with `Has Duplicates` labelEscalation ResolvedThis issue's escalations have been approved/rejectedHighA valid High severity issueRewardA payout will be made for this issue

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions