Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Market using stETH might still be delinquent after repayDeliquentDebt #319

Open
c4-bot-10 opened this issue Sep 18, 2024 · 0 comments
Open
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working 🤖_primary AI based primary recommendation 🤖_71_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality

Comments

@c4-bot-10
Copy link
Contributor

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 :

  • The easiest one and that do not require to modify the code is to use wstETH rather than stETH
  • Ensure the correct amount has been received by measuring the difference in balanceOf before and after the call. Although, the 1-2 wei issue could happen again
  • If the asset is stETH, transfer amount + a buffer to cover this case

Assessed type

ERC20

@c4-bot-10 c4-bot-10 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Sep 18, 2024
c4-bot-10 added a commit that referenced this issue Sep 18, 2024
@c4-bot-12 c4-bot-12 added 🤖_71_group AI based duplicate group recommendation 🤖_primary AI based primary recommendation labels Sep 18, 2024
howlbot-integration bot added a commit that referenced this issue Sep 20, 2024
@howlbot-integration howlbot-integration bot added the sufficient quality report This report is of sufficient quality label Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working 🤖_primary AI based primary recommendation 🤖_71_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

2 participants