The Wildcat Protocol - nobody2018's results

Lines of code

 https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketToken.sol#L64-L82

Vulnerability details

Impact

When an account is sanctioned on Chainalysis,

1. Anyone can call WildcatMarketConfig.nukeFromOrbit to set account.approval to AuthRole.Blocked and transfer account.scaledBalance to the corresponding WildcatSanctionsEscrow.
2. executeWithdrawal is called to execute a pending withdrawal request for the account. This transfers both the account's asset and market token into their respective WildcatSanctionsEscrow.

Until the account is canceled sanction or the borrower overrides the sanction status, the account can get back the asset locked in WildcatSanctionsEscrow. However, when an account is sanctioned by Chainalysis, changing account.approval to AuthRole.Blocked needs to be triggered manually (the two functions mentioned above). Before account.approval is changed, the account can call ERC20.transfer to transfer all market tokens it holds to a new account, which does not need to be authorized by the borrower. Afterwards, call WildcatMarketController.updateLenderAuthorization to set the new account's role to AuthRole.WithdrawOnly. Finally, calling queueWithdrawal creates a withdrawal request. Because the new account has not been sanctioned, call executeWithdrawal to execute a pending withdrawal request to obtain the asset after the batch expires.

The above bypass breaks the core assumption of the protocol: a sanctioned account can only wait until the sanction is lifted or the borrower manually overwrites the sanction status.

Proof of Concept

 Someone can run a program to continuously monitor whether the specified account has been sanctioned. Once it is detected that the specified account has been sanctioned, all market token of the account will be transferred to a new address via trasnfer.

File: src\market\WildcatMarketToken.sol
36:   function transfer(address to, uint256 amount) external virtual nonReentrant returns (bool) {
37:->   _transfer(msg.sender, to, amount);
38:     return true;
39:   }
......
64:   function _transfer(address from, address to, uint256 amount) internal virtual {
65:     MarketState memory state = _getUpdatedState();
66:     uint104 scaledAmount = state.scaleAmount(amount).toUint104();
67: 
68:     if (scaledAmount == 0) {
69:       revert NullTransferAmount();
70:     }
71: 
72:->   Account memory fromAccount = _getAccount(from);
73:     fromAccount.scaledBalance -= scaledAmount;
74:     _accounts[from] = fromAccount;
75: 
76:     Account memory toAccount = _getAccount(to);
77:     toAccount.scaledBalance += scaledAmount;
78:     _accounts[to] = toAccount;
79: 
80:     _writeState(state);
81:     emit Transfer(from, to, amount);
82:   }

L72, _getAccount(from) is used to obtain the corresponding account information. As long as from's account.approval is not AuthRole.Blocked, tx will execute successfully.
Since the account is currently sanctioned by Chainalysis, this status has not yet been synchronized to the Market, which needs to be triggered manually (described in the Impact section).

Consider the following scenario:

As a borrower, Alice deploys a WildcatMarket (M1).

1. Alice calls WildcatMarketController.authorizeLenders to authorize bob’s address (B1).
2. bob wrote a program to monitor the ChainalysisSanctionsList contract in order to track whether B1 has been sanctioned.
3. bob calls M1.deposit to deposit some assets and obtain the corresponding M1 token.
4. As time goes by, bob earns a lot of interest. However, unfortunately, B1 was sanctioned by the ChainalysisSanctionsList contract.
5. bob's program monitors this event and immediately initiates a call to M1.transfer/transferFrom to transfer all M1 tokens held by B1 to B2. B2 is not authorized by Alice.
6. bob calls M1.updateLenderAuthorization(B2, markets), where markets contains M1. This will set B2's account.approval to AuthRole.WithdrawOnly.
7. bob creates a withdrawal request for B2 via M1.queueWithdrawal which only requires AuthRole.WithdrawOnly.
8. Over time, the batch expires.
9. bob calls M1.executeWithdrawal to execute a pending withdrawal request for B2. bob successfully obtains asset(including interest).

The above scenario describes how bob bypasses sanction restrictions.

Tools Used

Manual Review

Recommended Mitigation Steps

File: src\market\WildcatMarketToken.sol
64:   function _transfer(address from, address to, uint256 amount) internal virtual {
......
68:     if (scaledAmount == 0) {
69:       revert NullTransferAmount();
70:     }
71:+    require(!IWildcatSanctionsSentinel(sentinel).isSanctioned(borrower, from), "banned transfer");
72:     Account memory fromAccount = _getAccount(from);
73:     fromAccount.scaledBalance -= scaledAmount;
......
82:   }

Assessed type

Context

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketBase.sol#L172-L176 https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketWithdrawals.sol#L166-L170

Vulnerability details

Impact

When an account is sanctioned on Chainalysis, its assets will be transferred to the corresponding WildcatSanctionsEscrow. If an account wants to get back assets, it needs to meet one of the following conditions:

1. The account is no longer sanctioned on Chainalysis.
2. The borrower sets sanctionOverrides[borrower][account] to true via overrideSanction.

However, the current implementation passes in wrong parameters when creating WildcatSanctionsEscrow([1], [2]). Market's borrower becomes WildcatSanctionsEscrow's account while Market's account becomes WildcatSanctionsEscrow's borrower. This will have the following impacts:

• Market’s account cannot get back asset in WildcatSanctionsEscrow.
• Market’s borrower can immediately take away the assets in WildcatSanctionsEscrow.

Proof of Concept

We will analyze the flow of _blockAccount to prove this problem.

File: src\market\WildcatMarketBase.sol
163:   function _blockAccount(MarketState memory state, address accountAddress) internal {
164:     Account memory account = _accounts[accountAddress];
165:     if (account.approval != AuthRole.Blocked) {
166:       uint104 scaledBalance = account.scaledBalance;
167:       account.approval = AuthRole.Blocked;
168:       emit AuthorizationStatusUpdated(accountAddress, AuthRole.Blocked);
169: 
170:       if (scaledBalance > 0) {
171:         account.scaledBalance = 0;
172:         address escrow = IWildcatSanctionsSentinel(sentinel).createEscrow(
173:->         accountAddress,
174:->         borrower,
175:           address(this)
176:         );
177:         emit Transfer(accountAddress, escrow, state.normalizeAmount(scaledBalance));
178:->       _accounts[escrow].scaledBalance += scaledBalance;
179:         emit SanctionedAccountAssetsSentToEscrow(
180:           accountAddress,
181:           escrow,
182:           state.normalizeAmount(scaledBalance)
183:         );
184:       }
185:       _accounts[accountAddress] = account;
186:     }
187:   }

This function is divided into two steps:

1. Set account.approval to AuthRole.Blocked,
2. If account.scaledBalance is greater than 0, create the corresponding escrow and transfer the Market Token to the escrow (at L178).

The problem lies in L173 and L174: L173 should be borrower, and L174 should be accountAddress. Let's look at the code of WildcatSanctionsSentinel.createEscrow:

File: src\WildcatSanctionsSentinel.sol
095:   function createEscrow(
096:     address borrower,
097:     address account,
098:     address asset
099:   ) public override returns (address escrowContract) {
......
104:     escrowContract = getEscrowAddress(borrower, account, asset);
105: 
106:     if (escrowContract.codehash != bytes32(0)) return escrowContract;
107: 
108:->   tmpEscrowParams = TmpEscrowParams(borrower, account, asset);
109: 
110:     new WildcatSanctionsEscrow{ salt: keccak256(abi.encode(borrower, account, asset)) }();
......
119:   }

The TmpEscrowParams at L108 is prepared for the constructor of WildcatSanctionsEscrow. When the account is not sanctioned by Chainalysis or the borrower overrides the sanction status, releaseEscrow can be called to transfer the asset to the account.

File: src\WildcatSanctionsEscrow.sol
33:   function releaseEscrow() public override {
34:     if (!canReleaseEscrow()) revert CanNotReleaseEscrow();
35: 
36:     uint256 amount = balance();
37: 
38:->   IERC20(asset).transfer(account, amount);
39: 
40:     emit EscrowReleased(account, asset, amount);
41:   }

Through the above analysis, we can see that accountAddress of L173 in _blockAccount is eventually set to borrower of WildcatSanctionsEscrow, and borrower of L174 is set to account of WildcatSanctionsEscrow. This means that account of WildcatSanctionsEscrow is not sanctionedand and releaseEscrow() can immediately be called to transfer the asset to account, who is actually borrower of Market.

Tools Used

Manual Review

Recommended Mitigation Steps

There are two places in the code that need to be fixed:

File: src\market\WildcatMarketBase.sol
163:   function _blockAccount(MarketState memory state, address accountAddress) internal {
......
172:         address escrow = IWildcatSanctionsSentinel(sentinel).createEscrow(
173:-          accountAddress,
173:+          borrower,
174:-          borrower,
174:+          accountAddress,
175:           address(this)
176:         );
......
187:   }

File: src\market\WildcatMarketWithdrawals.sol
137:   function executeWithdrawal(
138:     address accountAddress,
139:     uint32 expiry
140:   ) external nonReentrant returns (uint256) {
......
164:     if (IWildcatSanctionsSentinel(sentinel).isSanctioned(borrower, accountAddress)) {
165:       _blockAccount(state, accountAddress);
166:       address escrow = IWildcatSanctionsSentinel(sentinel).createEscrow(
167:-        accountAddress,
167:+        borrower,
168:-        borrower,
168:+        accountAddress,
169:         address(asset)
170:       );
......
188:   }

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

The borrower can grant authorization for a set of lenders via WildcatMarketController.authorizeLenders. When an account is set as an authorized lender by a borrower, the account can normally deposit assets via WildcatMarket.deposit. This is expected behavior. A malicious user can front-run authorizeLenders called by borrower to set account.approval to AuthRole.WithdrawOnly, which is lower than AuthRole.DepositAndWithdraw. However, WildcatMarket.deposit requires the caller to have the AuthRole.DepositAndWithdraw role, which is checked internally. If the role of msg.sender is insufficient, tx will be revert.

Proof of Concept

 deposit internally calls _getAccountWithRole to check whether the caller has the AuthRole.DepositAndWithdraw role.

File: src\market\WildcatMarket.sol
59:     // Transfer deposit from caller
60:     asset.safeTransferFrom(msg.sender, address(this), amount);
61: 
62:     // Cache account data and revert if not authorized to deposit.
63:->   Account memory account = _getAccountWithRole(msg.sender, AuthRole.DepositAndWithdraw);
64:     account.scaledBalance += scaledAmount;

The code for _getAccountWithRole is as follows:

File: src\market\WildcatMarketBase.sol
197:   function _getAccountWithRole(
198:     address accountAddress,
199:     AuthRole requiredRole
200:   ) internal returns (Account memory account) {
201:     account = _getAccount(accountAddress);
202:     // If account role is null, see if it is authorized on controller.
203:     if (account.approval == AuthRole.Null) {
204:       if (IWildcatMarketController(controller).isAuthorizedLender(accountAddress)) {
205:         account.approval = AuthRole.DepositAndWithdraw;
206:         emit AuthorizationStatusUpdated(accountAddress, AuthRole.DepositAndWithdraw);
207:       }
208:     }
209:     // If account role is insufficient, revert.
210:->   if (uint256(account.approval) < uint256(requiredRole)) {
211:->     revert NotApprovedLender();
212:     }
213:   }

L201, call _getAccount(accountAddress) to obtain the corresponding Account structure. If accountAddress already has the AuthRole.Blocked role, then the function will revert internally.

L203-208, if the account is a new account and has been authorized by the borrower, then account.approval will be updated from AuthRole.Null to AuthRole.DepositAndWithdraw. If account.approval is not equal to AuthRole.Null, then the code here will be skipped.

L210, check that account.approval will not be lower than requiredRole. Otherwise, tx will revert.

As can be seen from the above code, if account.approval is not AuthRole.Null and cannot be less than requiredRole (here equal to AuthRole.DepositAndWithdraw).

Anyone can call WildcatMarketController.updateLenderAuthorization, which will internally call WildcatMarket(market).updateAccountAuthorization to set the role of the specified account. If an account has not been authorized by the borrower, then the account's role can be set to AuthRole.WithdrawOnly. In this way, after the account is authorized, he calls the deposit function to deposit the asset, and tx will be revert.

Consider the following scenario:

As a borrower, Alice deploys a WildcatMarket (M1).

1. Alice calls WildcatMarketController.authorizeLenders to authorize bob, tx1 enters the memory pool.
2. Tom notices tx1 and immediately initiates updateLenderAuthorization(bob, markets) to front-run tx1. tx2 enters the memory pool. The markets array contains M1.
3. tx2 is executed, which means bob has not been authorized yet, so bob’s account.approval is set to AuthRole.WithdrawOnly.
4. tx1 is executed and bob is authorized.
5. bob calls deposit to deposit some assets. However, since bob's account.approval is AuthRole.WithdrawOnly(2) which is smaller than AuthRole.DepositAndWithdraw(3), tx revert.

Tools Used

Manual Review

Recommended Mitigation Steps

File: src\market\WildcatMarketBase.sol
197:   function _getAccountWithRole(
198:     address accountAddress,
199:     AuthRole requiredRole
200:   ) internal returns (Account memory account) {
201:     account = _getAccount(accountAddress);
202:     // If account role is null, see if it is authorized on controller.
203:-    if (account.approval == AuthRole.Null) {
203:+    if (account.approval == AuthRole.Null || account.scaledBalance == 0) {
204:       if (IWildcatMarketController(controller).isAuthorizedLender(accountAddress)) {
205:         account.approval = AuthRole.DepositAndWithdraw;
206:         emit AuthorizationStatusUpdated(accountAddress, AuthRole.DepositAndWithdraw);
207:       }
208:     }
209:     // If account role is insufficient, revert.
210:     if (uint256(account.approval) < uint256(requiredRole)) {
211:       revert NotApprovedLender();
212:     }
213:   }

If _getAccount(accountAddress) at L201 returns normally, it means that accountAddress’s role will not be AuthRole.Blocked.

Assessed type

DoS