The Wildcat Protocol - ke1caM's results

Banking, but worse - a protocol for fixed-rate, undercollateralised credit facilities.

General Information

Platform: Code4rena

Start Date: 16/10/2023

Pot Size: $60,500 USDC

Total HM: 16

Participants: 131

Period: 10 days

Judge: 0xTheC0der

Total Solo HM: 3

Id: 296

League: ETH

Wildcat Protocol

Findings Distribution

Researcher Performance

Rank: 86/131

Findings: 2

Award: $10.23

QA:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarket.sol#L142-L162 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L136-L139

Vulnerability details

Summary

Borrower can not close the market due to missing implementation of closing market mechanism in WildcatMarketController.sol. This unables borrower to ever cancel the loan.

Vulnerability details

In WildcatMarket there is a function closeMarket which is responsilbe for closing the market, setting APR to 0%, transfering remaining debts from borrower and finally marking the market as closed. In other words it allows the borrower to repay his obligations and quit the loan if he ever wishes to. The closeMarket function has a flaw. It can be only called by WildcatMarketController contract but there is no such functionality in this smart contract.

Let's take a closer look at closeMarket function and it's modifiers.

function closeMarket() external onlyController nonReentrant {
    MarketState memory state = _getUpdatedState();
    state.annualInterestBips = 0;
    state.isClosed = true;
    state.reserveRatioBips = 0;
    if (_withdrawalData.unpaidBatches.length() > 0) {
      revert CloseMarketWithUnpaidWithdrawals();
    }
    uint256 currentlyHeld = totalAssets();
    uint256 totalDebts = state.totalDebts();
    if (currentlyHeld < totalDebts) {
      // Transfer remaining debts from borrower
      asset.safeTransferFrom(borrower, address(this), totalDebts - currentlyHeld);
    } else if (currentlyHeld > totalDebts) {
      // Transfer excess assets to borrower
      asset.safeTransfer(borrower, currentlyHeld - totalDebts);
    }
    _writeState(state);
    emit MarketClosed(block.timestamp);
  }

As we can see there is onlyController modifier which ensures that ONLY the controller can call this function.

modifier onlyController() {
    if (msg.sender != controller) revert NotController();
    _;
  }

Borrower can't close the market because only controller can call this function but the implementation for calling this function is missing in the WildcatMarketController.

PoC

If we try to call this function directly in WildcatMarket the transaction will revert. There is no other way to call this function because there is no such functionality in WildcatMarketController.

Copy and paste this test into the WildcatMarket.t.sol at the closeMarket() section.

function test_borrower_cant_close_market() external {
        _depositBorrowWithdraw(alice, 1e18, 8e17, 1e18);
        startPrank(borrower);
        asset.approve(address(market), 8e17);
        vm.expectRevert(IMarketEventsAndErrors.NotController.selector);
        market.closeMarket();
        stopPrank();
    }

Impact

Borrower can't close the market, he won't be able to quit the loan and this means loss of funds for the borrower.

Tools used

VScode, Manual Review, Foundry

Recommendations

Implement functionality in WildcatMarketController to allow borrower to close the market or change the modifier to onlyBorrower. This will allow borrower to call closeMarket directly in WildcatMarket.

Assessed type

Access Control

#0 - c4-pre-sort

2023-10-27T07:25:01Z

minhquanym marked the issue as duplicate of #147

#1 - c4-judge

2023-11-07T13:53:20Z

MarioPoneder changed the severity to 2 (Med Risk)

#2 - c4-judge

2023-11-07T14:05:35Z

MarioPoneder marked the issue as partial-50

#3 - c4-judge

2023-11-07T14:16:53Z

MarioPoneder changed the severity to 3 (High Risk)

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketConfig.sol#L134-L144

Vulnerability details

Summary

setMaxTotalSupply is implemented in WildcatMarketConfig and can only be called by the controller contract (WildcatMarketController). However controller contract lacks functionality to enable this operation to borrower.

Vulnerability details

Max total supply is a limit value for deposits. Borrower has to pay an interest based on the amount deposited by the lenders. If borrower would like to change the max total supply (to lower the deposit amount and paid interest in future) he is unable to do so. It is worth acknowledging that new max total supply can not be lower than the current deposits but borrower could just reevalute his decision about the token amount that he wishes to borrow and lower this amount just to prevent future deposits.

PoC / Scenario

Let's say that user would like to borror 1000 USDC and set max total supply to 1000 USDC. After rethinking his decision we decides that he don't want to borrow this much USDC and he would like to set new max total supply to 500 USDC. It is impossible since the WildcatMarketController lacks functionality to do so and it means that lenders can deposit the 1000 USDC and borrower can not to anything to change that.

    function test_borrower_cant_change_maxTotalSupply() external {
        vm.startPrank(borrower);
        vm.expectRevert(IMarketEventsAndErrors.NotController.selector);
        // try to call setMaxTotalSupply directly since there is no such functionality in controller
        market.setMaxTotalSupply(500);
        vm.stopPrank();
    }

We try to call setMaxTotalSupply directly in WildcatMarketConfig but the transaction reverts. It means that borrower is unable to set new maxTotalSupply.

Impact

Loss of funds for borrower.

Tools used

VScode, Manual Review, Foundry

Recommendations

Implement a function in WildcatMarketController which allows borrower to change the maxTotalSupply value.

Assessed type

Access Control

#0 - c4-pre-sort

2023-10-27T07:00:11Z

minhquanym marked the issue as duplicate of #147

#1 - c4-judge

2023-11-07T13:53:20Z

MarioPoneder changed the severity to 2 (Med Risk)

#2 - c4-judge

2023-11-07T13:55:46Z

MarioPoneder marked the issue as partial-50

#3 - c4-judge

2023-11-07T14:16:53Z

MarioPoneder changed the severity to 3 (High Risk)

Awards

10.1663 USDC - $10.17

Labels

bug
downgraded by judge
grade-b
low quality report
QA (Quality Assurance)
Q-26

External Links

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatMarketController.sol#L153-L160 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatMarketController.sol#L182-L190

Vulnerability details

Summary

When borrower authorizes lenders anyone can later add them to any market that the borrower has created or will create in future even when borrower does not want a particular lender in a specific market.

Vulnerability details

When borrower authorizes lender in authorizeLenders they are added to _authorizedLenders. To update the authorization someone needs to call updateLenderAuthorization. This function can be called by anyone. The only restrictions are that the passed market must be added to the _controlledMarkets and the lender is in _authorizedLenders. So when the lender is authorized he can be added to any market, by anyone, without borrower consent.

Scenario

  1. Let's say that borrower has two markets. USDC with 5% APR and WETH with 7% APR.
  2. He agreed with two different lenders for a specific terms and authorizes them for the specific markets using authorizeLenders and updateLenderAuthorization.
  3. The first lenders (USDC 5%) sees that he could get higher APR in WETH market so he calls udpateLenderAuthorization for WETH market and can deposit and withdraw his tokens on that specific market even though the borrower would like this market to be specificly for lender number 2.

This is only an example scenario of how this flaw could affect the borrowers market. There could be another ways to utilize this logic which might be more sophisticated than this one.

Impact

This flaw restricts borrowers flexibility in terms of authorization specific lenders for specific markets.

Tools used

VScode, Manual Review

Recommendations

Allow borrower to choose what lenders are allowed to interact with specific markets by adding onlyBorrower modifier in updateLenderAuthorization or create a new mechanism that allows borrower to authorize specific lender for a specific market.

Assessed type

Access Control

#0 - c4-pre-sort

2023-10-28T12:03:50Z

minhquanym marked the issue as low quality report

#1 - minhquanym

2023-10-28T12:04:01Z

More like a feature instead of issue. Consider QA

#2 - c4-judge

2023-11-08T18:34:42Z

MarioPoneder changed the severity to QA (Quality Assurance)

#3 - c4-judge

2023-11-09T14:56:52Z

MarioPoneder marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter