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 #25

Closed
howlbot-integration bot opened this issue Sep 20, 2024 · 3 comments
Closed
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 issue Highest quality submission among a set of duplicates 🤖_primary AI based primary recommendation 🤖_71_group AI based duplicate group recommendation sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@howlbot-integration
Copy link

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

@howlbot-integration howlbot-integration bot added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value 🤖_71_group AI based duplicate group recommendation 🤖_primary AI based primary recommendation bug Something isn't working primary issue Highest quality submission among a set of duplicates sufficient quality report This report is of sufficient quality labels Sep 20, 2024
howlbot-integration bot added a commit that referenced this issue Sep 20, 2024
@laurenceday
Copy link

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.

@laurenceday laurenceday added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Sep 20, 2024
@3docSec
Copy link

3docSec commented Oct 4, 2024

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
Copy link
Contributor

c4-judge commented Oct 4, 2024

3docSec marked the issue as unsatisfactory:
Out of scope

@c4-judge c4-judge closed this as completed Oct 4, 2024
@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Oct 4, 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 issue Highest quality submission among a set of duplicates 🤖_primary AI based primary recommendation 🤖_71_group AI based duplicate group recommendation sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

3 participants