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: 100/131
Findings: 2
Award: $6.73
🌟 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
Function closeMarket in the WildcatMarket.sol can't be called by anyone which will make all of the markets not able to be closed.
The function closeMarket should be called to close a market, which will make the borrower who created the market transfer the remaining debt of the market to the contract, as can be seen here
https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarket.sol#L152-L154
or in the case where there is no debt and the contract holds more funds than it should, it will transfer the excess to him
https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarket.sol#L155-L158
The problem relies in the fact that the function closeMarket uses the onlyController which means that only the controller which created the market can call this function, as can be seen here
https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L136-L139
but there are no instances in the WildcatMarketController.sol contract that calls closeMarket function. Basically the function closeMarket is not called anywhere in any contract, which makes the function basically unusable and useless, making impossible to close any market.
Manual review
Instead of onlyController use the onlyBorrower, to let only the borrower close the market, or create an instance in the WildcatMarketController.sol that calls closeMarket function, so the call will not revert all the time.
Error
#0 - c4-pre-sort
2023-10-27T07:24:15Z
minhquanym marked the issue as duplicate of #147
#1 - c4-judge
2023-11-07T13:53:21Z
MarioPoneder changed the severity to 2 (Med Risk)
#2 - c4-judge
2023-11-07T14:05:21Z
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
Function setMaxTotalSupply in the WildcatMarket.sol can't be called by anyone which will make the max total supply of any market not be able to be changed.
The function setMaxTotalSupply should be called to change the maxTotalSupply of a market, in the case where the borrower intends to let more assets to be deposited to the market than initially planned. The function uses onlyController
https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketConfig.sol#L134
which lets only the controller that created the market call the function
https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L136-L139
The problem relies in the fact that the function setMaxTotalSupply is not called anywhere in the WildcatMarketController.sol and nor in any other contract, which makes the function basically useless since it can't be called by anyone, and any attempts will lead to a revert.
Manual review
Instead of onlyController use the onlyBorrower modifier, or create an instance in the WildcatMarketController.sol where setMaxTotalSupply is called, so the borrowers can change the maxTotalSupply as intended.
Other
#0 - c4-pre-sort
2023-10-27T06:56:08Z
minhquanym marked the issue as duplicate of #162
#1 - c4-pre-sort
2023-10-27T06:58:18Z
minhquanym marked the issue as duplicate of #147
#2 - c4-judge
2023-11-07T13:50:28Z
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: YusSecurity
Also found by: 0xAsen, 0xCiphky, 0xDING99YA, 0xKbl, 0xSwahili, 0xbepresent, 3docSec, AS, Aymen0909, DeFiHackLabs, GREY-HAWK-REACH, KeyKiril, MiloTruck, QiuhaoLi, Silvermist, SovaSlava, TrungOre, VAD37, Vagner, Yanchuan, ZdravkoHr, ast3ros, cartlex_, d3e4, deth, ggg_ttt_hhh, gizzy, kodyvim, nirlin, nobody2018, rvierdiiev, serial-coder, sl1, tallo, xeros
6.6715 USDC - $6.67
https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L172-L176 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketWithdrawals.sol#L166-L170
Arguments provided in the createEscrow called in the WildcatMarket.sol are in the wrong order, which will cause the assets kept in the escrow to return to the borrower instead of the lender, which will mean that the lender will lose all the funds.
The createEscrow function in WildcatSanctionsSentinel.sol is used to create an escrow contract where funds of lender are kept in the case where the lender gets sanctioned and the function has 3 address arguments as can be seen here
https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L95-L99
borrower, account and asset. Those 3 arguments are latter saved in the tmpEscrowParams in this order
https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L108
so they can be used by the escrow contract in the constructor
https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsEscrow.sol#L18
The problem relies in the fact that if you look at the WildcatMarket.sol in the two situation where createEscrow is called, the arguments provided are in the wrong order, instead of providing the borrower first and the account second, the account is provided first and the borrower second, as can be seen here
https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketWithdrawals.sol#L166-L170
https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L172-L176
This will lead to the borrower and account variables in WildcatSanctionsEscrow.sol to be swapped, since the order that they will be stored in tmpEscrowParams
https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L108
will be reversed. Because of that, the two important functions in the WildcatSanctionsEscrow.sol will not be accurate, since canReleaseEscrow
https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsEscrow.sol#L26
function will check if the borrower is sanctioned, not the account, and in the end releaseEscrow function
https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsEscrow.sol#L33-L41
will transfer the funds to the borrower address, not the lender address, as it is supposed to as written in the documentation
https://wildcat-protocol.gitbook.io/wildcat/using-wildcat/day-to-day-usage/the-sentinel
which will lead to loss of funds to the lender.
Manual review
In those two instances where createEscrow is called, use the borrower address as the first argument and the account as the second argument, to keep consistency between the arguments.
Other
#0 - c4-pre-sort
2023-10-27T02:52:58Z
minhquanym marked the issue as duplicate of #515
#1 - c4-judge
2023-11-07T12:09:18Z
MarioPoneder marked the issue as satisfactory