code-423n4 / 2024-08-wildcat-findings

3 stars 1 forks source link

Market using `stETH` might still be delinquent after `repayDeliquentDebt` #25

Closed howlbot-integration[bot] closed 1 month ago

howlbot-integration[bot] commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-08-wildcat/blob/main/src/market/WildcatMarket.sol#L188-L188 https://github.com/code-423n4/2024-08-wildcat/blob/main/src/market/WildcatMarketBase.sol#L541-L542

Vulnerability details

Summary

stETH is known for having an issue on transfer, causing the recipient to sometimes receive 1-2 wei less than expected. Thus, it is possible that the transfered amount in repayDeliquentDebt will not be sufficient to make the market non delinquent as expected by the call. ERC20 Token Behaviors In Scope set tokens where balance changes outside of transfers as In scope, which correspond to stETH

Vulnerability details

repayDeliquentDebt calculate the delinquentDebt as exactly the difference between the totalAssets() and the liquidityRequired(), which define if the market is delinquent

File: src/market/WildcatMarket.sol
186:   function repayDelinquentDebt() external nonReentrant sphereXGuardExternal {
187:     MarketState memory state = _getUpdatedState();
188:     uint256 delinquentDebt = state.liquidityRequired().satSub(totalAssets());
189:     _repay(state, delinquentDebt, 0x04);
190:     _writeState(state);
191:   }

Then amount = delinquentDebt is transfered from the caller to the market:

File: src/market/WildcatMarket.sol
168:   function _repay(MarketState memory state, uint256 amount, uint256 baseCalldataSize) internal {
169:     if (amount == 0) revert_NullRepayAmount();
170:     if (state.isClosed) revert_RepayToClosedMarket();
171: 
172:     asset.safeTransferFrom(msg.sender, address(this), amount);
173:     emit_DebtRepaid(msg.sender, amount);
174: 
175:     // Execute repay hook if enabled  //!hook
176:     hooks.onRepay(amount, state, baseCalldataSize);
177:   }

So, if the market receives 1-2 wei less than exepected, the liquidityRequired will still be greater than totalAssets() and the market will still be delinquent

File: src/market/WildcatMarketBase.sol
540:   function _writeState(MarketState memory state) internal {
541:     bool isDelinquent = state.liquidityRequired() > totalAssets();
542:     state.isDelinquent = isDelinquent; 
543:    // .... 
544: // ....    

Impact

Function not working as expected for stETH. This will cause the market to still accrue delinquency fees, causing a loss for the borrower.

Tools Used

Manual review

Recommended Mitigation Steps

There are multiple ways to solve this :

Assessed type

ERC20

laurenceday commented 2 months ago

The summary here contradicts itself:

stETH is known for having an issue on transfer ERC20 Token Behaviors In Scope set tokens where balance changes outside of transfers as In scope, which correspond to stETH

Notwithstanding that this finding literally involves a balance change relating to transfers (intentional or not), the repo specification points out that Creating markets for rebasing tokens breaks the underlying interest rate model in the Scoping Q&A, so stETH is out of scope anyway.

3docSec commented 1 month ago

Looking at the finding the behavior is consistent with the fee-on-transfer, not rebasing, behavior - and the former is out of scope.

c4-judge commented 1 month ago

3docSec marked the issue as unsatisfactory: Out of scope