Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

TokenManager contract is not compatible with Rebasing Token.

joshuajee

Summary

According to the contest details, any ERC20 token can be whitelisted as collateral and (ETH, USDC, LINK, ankrETH, etc) will be whitelisted as collateral, the problem here is that ankrETH is a rebasing token meaning balances of users and totalSupply can change over time. In the current state of the protocol, positive rebasing will lead to a loss of reward, while negative rebasing could make it impossible for some users to withdraw because their funds will be stuck in the protocol.

https://codehawks.cyfrin.io/c/2024-08-tadle

Vulnerability Details

The following function adds the tokens to the userTokenBalanceMap mappings, but for the case of rebasing tokens balanceOf()changes over time, so when it changes the userTokenBalanceMap won't match the real user's balance.

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/TokenManager.sol#L113

function addTokenBalance(
TokenBalanceType _tokenBalanceType,
address _accountAddress,
address _tokenAddress,
uint256 _amount
) external onlyRelatedContracts(tadleFactory, _msgSender()) {
@-> userTokenBalanceMap[_accountAddress][_tokenAddress][
_tokenBalanceType
] += _amount;
}

The function below withdraws the token from the userTokenBalanceMap, and it doesn't make any reference to the balanceOf the contract.

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/TokenManager.sol#L141

function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
@-> uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
...
​
{
_safe_transfer_from(
_tokenAddress,
capitalPoolAddr,
_msgSender(),
@-> claimAbleAmount
);
}
​
...
}

POC

For Positive Rebase

  1. Alice deposit 1000 ankrETH.

  2. Positive rebase occurs and 1000 ankrETH is now 1100 ankrETH.

  3. When she withdraws she will get 1000 ankrETH instead of 1100 ankrETH.

  4. The remaining 100 ankrETH will be stuck in the protocol.

For Negative Rebase

  1. Alice deposits 1000 tokens.

  2. Bob deposits 1000 tokens.

  3. A negative rebase occurs and 1000 tokens are now 900 tokens, so the contract currently holds 1800 tokens.

  4. When she withdraws she will get 1000 tokens instead of 900 tokens, so she gets 100 tokens more than what she is supposed to get.

  5. The remaining balance of the contract will be 800, so when Bob tries to withdraw, the transaction will fail because Bob 1000 tokens are stored on Bob's userTokenBalanceMap.

  6. So Bob is unable to get their money back.

Impact

  1. Loss of reward tokens for positive rebase, e.g in the case of ankrETH .

  2. When a negative rebase occurs, userTokenBalanceMap is going to be less than the actual balance the user should own, so if they withdraw they are going to earn more than they should and the protocol is going to lose a lot of funds that it will not be able to pay the last person that withdraws.

Tools Used

Manual Analysis

Recommendations

  1. Don't Whitelist rebasing tokens

  2. Only accept the wrapped version of rebasing tokens

  3. Use a token vault for collateral deposit (ERC4626).

Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-TokenManager-FOT-Rebasing

Valid medium, there are disruptions to the ability to take market actions. The following functions will be disrupted without the possibiliy of reaching settlement, since the respective offers cannot be created/listed regardless of mode when transferring collateral token required to the CapitalPool contract or when refunding token from user to capital pool during relisting. So withdrawal is not an issue - `createOffer()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L96-L102) - `listOffer()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L355-L362) - `relistOffer()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L515-L521) - `createTaker()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L831-L836) I believe medium severity is appropriate although the likelihood is high and impact is medium (only some level of disruption i.e. FOT tokens not supported and no funds at risk)

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.