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
Rank: 86/131
Findings: 2
Award: $10.23
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: 0xpiken
Also found by: 0xCiphky, 0xComfyCat, 0xStalin, 0xhegel, 0xkazim, 3docSec, AM, Aymen0909, CaeraDenoir, DeFiHackLabs, Drynooo, Eigenvectors, Fulum, HALITUS, HChang26, Jiamin, Juntao, LokiThe5th, Mike_Bello90, MiloTruck, QiuhaoLi, Silvermist, SovaSlava, SpicyMeatball, T1MOH, Toshii, TrungOre, TuringConsulting, Vagner, Yanchuan, ZdravkoHr, _nd_koo, almurhasan, audityourcontracts, ayden, cartlex_, circlelooper, crunch, cu5t0mpeo, deth, erictee, ggg_ttt_hhh, gizzy, gumgumzum, hash, jasonxiale, josephdara, ke1caM, kodyvim, lanrebayode77, marqymarq10, max10afternoon, nirlin, nonseodion, osmanozdemir1, peter, radev_sw, rvierdiiev, said, serial-coder, sl1, smiling_heretic, squeaky_cactus, stackachu, tallo, trachev, zaevlad
0.0606 USDC - $0.06
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
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.
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
.
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(); }
Borrower can't close the market, he won't be able to quit the loan and this means loss of funds for the borrower.
VScode, Manual Review, Foundry
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
.
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)
🌟 Selected for report: 0xpiken
Also found by: 0xCiphky, 0xComfyCat, 0xStalin, 0xhegel, 0xkazim, 3docSec, AM, Aymen0909, CaeraDenoir, DeFiHackLabs, Drynooo, Eigenvectors, Fulum, HALITUS, HChang26, Jiamin, Juntao, LokiThe5th, Mike_Bello90, MiloTruck, QiuhaoLi, Silvermist, SovaSlava, SpicyMeatball, T1MOH, Toshii, TrungOre, TuringConsulting, Vagner, Yanchuan, ZdravkoHr, _nd_koo, almurhasan, audityourcontracts, ayden, cartlex_, circlelooper, crunch, cu5t0mpeo, deth, erictee, ggg_ttt_hhh, gizzy, gumgumzum, hash, jasonxiale, josephdara, ke1caM, kodyvim, lanrebayode77, marqymarq10, max10afternoon, nirlin, nonseodion, osmanozdemir1, peter, radev_sw, rvierdiiev, said, serial-coder, sl1, smiling_heretic, squeaky_cactus, stackachu, tallo, trachev, zaevlad
0.0606 USDC - $0.06
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.
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.
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.
Loss of funds for borrower.
VScode, Manual Review, Foundry
Implement a function in WildcatMarketController
which allows borrower to change the maxTotalSupply
value.
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)
🌟 Selected for report: J4X
Also found by: 0x3b, 0xCiphky, 0xComfyCat, 0xStalin, 0xbepresent, 3docSec, DavidGiladi, DeFiHackLabs, Drynooo, Fulum, GREY-HAWK-REACH, InAllHonesty, MatricksDeCoder, Mike_Bello90, MiloTruck, Phantom, SHA_256, T1MOH, Udsen, VAD37, YusSecurity, ZdravkoHr, ast3ros, caventa, deepkin, deth, devival, ggg_ttt_hhh, inzinko, jasonxiale, josieg_74497, karanctf, ke1caM, nisedo, nobody2018, nonseodion, osmanozdemir1, radev_sw, rvierdiiev, serial-coder, t0x1c
10.1663 USDC - $10.17
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
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.
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.
USDC
with 5% APR and WETH
with 7% APR.authorizeLenders
and updateLenderAuthorization
.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.
This flaw restricts borrowers flexibility in terms of authorization specific lenders for specific markets.
VScode, Manual Review
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.
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