Submitted by YusSecurity, also found by d3e4, serial-coder, nirlin, xeros, VAD37, DeFiHackLabs, 0xKbl, 0xDING99YA, Aymen0909, ast3ros, Yanchuan, MiloTruck, AS, sl1, QiuhaoLi, gizzy, SovaSlava, TrungOre, cartlex_, kodyvim, Vagner, KeyKiril, GREY-HAWK-REACH, 0xSwahili, ggg_ttt_hhh, 3docSec, Silvermist, 0xbepresent, tallo, ZdravkoHr, 0xCiphky, rvierdiiev, nobody2018, deth, and 0xAsen
https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L96-L97
https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L173-L174
https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketWithdrawals.sol#L166-L170
The WildcatMarketBase#_blockAccount() function that is used to block a sanctioned lender contains a critical bug. It incorrectly calls IWildcatSanctionsSentinel(sentinel).createEscrow() with misordered arguments, accidentally creating a vulnerable escrow that enables the borrower to drain all the funds of the sanctioned lender.
The execution of withdrawals (WildcatMarketWithdrawals#executeWithdrawal()) also performs a check if the accountAddress is sanctioned and if it is, then escrow is created and the amount that was to be sent to the lender is sent to the escrow. That escrow, however, is also created with the account and borrower arguments in the wrong order.
That means whether or not the borrower has anything to do with a sanctioned account and their funds ever, that account will never be able to get their money back in case their sanction gets dismissed.
Consider this scenario to illustrate how the issue can be exploited:
Now, lets delve into the specifics and mechanics of the vulnerability:
The nukeFromOrbit() function calls _blockAccount(state, larryAddress), blocking Larrys account, creating an escrow, and transferring his market tokens to that escrow.
In the code snippet, notice the order of arguments passed to createEscrow():
However, when we examine the WildcatSanctionsSentinel#createEscrow() implementation, we see a different order of arguments. This results in an incorrect construction of tmpEscrowParams:
The tmpEscrowParams are essential for setting up the escrow correctly. They are fetched in the constructor of WildcatSanctionsEscrow, and the order of these parameters is significant:
However, due to the misordered arguments in _blockAccount(), whats passed as tmpEscrowParams is (borrower = Larry, account = Bob, asset), which is incorrect. This misordering affects the canReleaseEscrow() function, which determines whether releaseEscrow() should proceed or revert.
The misordered parameters impact the return value of sentinel.isSanctioned(). It mistakenly checks Bob against the sanctions list, where he is not sanctioned.
Thus isSanctioned() returns false and consequently canReleaseEscrow() returns true. This allows Bob to successfully execute releaseEscrow() and drain all of Larrys market tokens:
After this, Bob simply needs to authorize himself as a lender in his own market and withdraw the actual assets.
Below is a PoC demonstrating how to execute the exploit. To proceed, please include the following import statements in test/market/WildcatMarketConfig.t.sol:
Add the following test test/market/WildcatMarketConfig.t.sol as well:
Run the PoC like this:
Fix the order of parameters in WildcatSanctionsSentinel#createEscrow(borrower, account, asset):
Error
laurenceday (Wildcat) commented:
laurenceday (Wildcat) confirmed
