The Wildcat Protocol - gizzy's results

Lines of code

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

Vulnerability details

Impact

Market can not be closed because it contains onlyController modifier but the controller does not have any function to close market. This makes the borrower to always pay interest with having any way to close market when he has repaid everything he owed.

Proof of Concept

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarket.sol#L142C3-L162C2

The close function contains the onlyController modifier but through out the code of WildcatMarketController there is not function to closed market making it always open and infinite interest being paid even after all repayment

Tools Used

Mauel review

Recommended Mitigation Steps

it should probably be onlyBorrower or a function is introduced in the controller and passes through

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketWithdrawals.sol#L77

Vulnerability details

Impact

Any other person can withdraw from a market which breaks protocol Invariants which states that

The assets of a market should never be able to be withdrawn by anyone that is not the borrower or a lender with either the WithdrawOnly or DepositAndWithdraw role.
Exceptions: balances being transferred to a blocked account's escrow contract and collection of protocol fees.

This is possible because the lender can transfer his market tokens to any address and update his role to withdraw only

Proof of Concept

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatMarketController.sol#L182C2-L190C4

The updateLenderAuthorization is callable by any one which gives any specified address the withdrawOnly role if the address is not added by the borrower. So this makes it possible for anyone to withdraw from market .

test in test/market/wildcatmarketwithdrawals.t.sol

  function test_queueWithdrawal_AuthorizedWithdrawOnly() public asAccount(bob) {
    address[] memory markets = new address[](1);
    markets[0] = address(market);
    _deposit(bob, 1e18);
    market.transfer(address(2), 1e18);
    startPrank(address(2));
    controller.updateLenderAuthorization(address(2), markets);
    market.queueWithdrawal(1e18);
    stopPrank();
  }

Tools Used

manuel review

Recommended Mitigation Steps

make the function restricted to only borrower.

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L172

Vulnerability details

Impact

there is a wrong placement of address when creating escrow which will return funds to borrower. when creating ecrow its first input should be the borrowers address and then then account for escrow but this was misplaced which will lead to the borrowers being set as account and so can withdraw all funds sent to escrow

Proof of Concept

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L95C2-L99C55 from the creating of escrow above you will find out that the borrowers address comes first before the account and this is exactly how it is recorded is ecrow. The problem is the blockaccount https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L172C7-L176C11 places the borrower as the second input which makes the borrower the account and the account the borrower meaning that the borrower can easily claim all funds in the escrow .

From test/market/wildcatmarketconfig.t.sol compare the alice address which should be the escrow onwer to which funds are sent to and the actual value stored as account in escrow which is the borrower instead of the alice

  function test_nukeFromOrbit_WithBalance() external {
    _deposit(alice, 1e18);
    address escrow = sanctionsSentinel.getEscrowAddress(
      alice,
      borrower,
      address(market)
    );
    sanctionsSentinel.sanction(alice);

    vm.expectEmit(address(market));
    emit AuthorizationStatusUpdated(alice, AuthRole.Blocked);
    vm.expectEmit(address(market));
    emit Transfer(alice, escrow, 1e18);
    vm.expectEmit(address(market));
    emit SanctionedAccountAssetsSentToEscrow(alice, escrow, 1e18);
    market.nukeFromOrbit(alice);

    console2.log(
      "account address",
      WildcatSanctionsEscrow(escrow).account(),
      alice,
      borrower
    );

    assertEq(
      uint(market.getAccountRole(alice)),
      uint(AuthRole.Blocked),
      "account role should be Blocked"
    );
  }

Tools Used

Manuel review

Recommended Mitigation Steps

just swap the addresses .

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatMarketController.sol#L468

Vulnerability details

Impact

The setAnnualInterestBips is used by borrower to reset the interest rate he pays to any other of his choice but since the protocol implements a constraint on the interest the borrower choice must be within the min and max AnnualInterestBips but this is only checked during creation of market , a borrower can reset his AnnualInterestBips to 0% using setAnnualInterestBips in WildcatMarketController even when the min AnnualInterestBips set by protocol is above 0 or if the max AnnualInterestBips is less than the BIPs

Proof of Concept

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatMarketController.sol#L468C2-L488C4

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

the market only checks if it higher than the BIPs and not max AnnualInterestBips and does not check of it is lower than minAnnualInterestBips

Tools Used

Manuel review

Recommended Mitigation Steps

Always check if the new AnnualInterestBips is less or higher than min and max AnnualInterestBips

Assessed type

Invalid Validation