Protocol markets are incompatible with rebasing tokens

[c4-submissions]

Lines of code

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

Vulnerability details

Bug Description

Some tokens (eg. AMPL), known as rebasing tokens, have dynamic balances. This means that the token balance of an address could increase or decrease over time.

However, markets in the protocol are unable to handle such changes in token balance. When lenders call depositUpTo(), the amount of assets they deposit is stored as a fixed amount in account.scaledBalance:

WildcatMarket.sol#L56-L65

    uint104 scaledAmount = state.scaleAmount(amount).toUint104();
    if (scaledAmount == 0) revert NullMintAmount();

    // Transfer deposit from caller
    asset.safeTransferFrom(msg.sender, address(this), amount);

    // Cache account data and revert if not authorized to deposit.
    Account memory account = _getAccountWithRole(msg.sender, AuthRole.DepositAndWithdraw);
    account.scaledBalance += scaledAmount;
    _accounts[msg.sender] = account;

Afterwards, when lenders want to withdraw their assets, the amount of assets that they can withdraw will be based off this value.

Therefore, since a lender's scaledBalance is fixed and does not change according to the underlying asset balance, lenders will lose funds if they deposit into a market with a rebasing token as the asset.

For example, if AMPL is used as the market's asset, and AMPL rebases to increase the token balance of all its users, lenders in the market will still only be able to withdraw the original amount they deposited multiplied by the market's interest rate. The underlying increase in AMPL will not accrue to anyone, and is only accessible by the borrower by calling borrow().

Impact

If a market uses a rebasing tokens as its asset, lenders will lose funds when the asset token rebases.

Recommended Mitigation

Consider implementing a token blacklist in the protocol, such as in WildcatArchController, and adding all rebasing tokens to this blacklist.

Additionally, consider documenting that markets are not compatible with rebasing tokens.

Assessed type

ERC20

[c4-pre-sort]

minhquanym marked the issue as primary issue

[c4-pre-sort]

minhquanym marked the issue as sufficient quality report

[minhquanym]

Consider QA

[laurenceday]

Agreed - this is a Low that we can flag up to borrowers deploying. Not interested in maintaining a token whitelist or blacklist, as it goes against the point that we don't intervene.

[c4-sponsor]

laurenceday marked the issue as disagree with severity

[c4-sponsor]

laurenceday (sponsor) acknowledged

[c4-judge]

MarioPoneder marked the issue as satisfactory

[c4-judge]

MarioPoneder marked issue #560 as primary and marked this issue as a duplicate of 560

[MarioPoneder]

1. I cannot find any info within the scope of this contest that rebasing tokens are OOS or unsupported.
   Please let me know, in case I overlooked it.
2. Rebasing tokens, like stETH to name a "central" example, are no novelty in the DeFi space anymore and users expect DeFi protocols to be compatible with them when no further notice is provided.

[c4-judge]

MarioPoneder marked the issue as selected for report

[0x3b33]

L-13 from the bot report explains that rebasing/fee-on-transfer tokens will not work.