Lines of code

Vulnerability details

Description

In collateral deposit() and withdraw() flow, a fee is calculated as a percentage of user’s requested amount. It is passed to the DepositHook and WithdrawHook, for example in deposit():

uint256 _amountAfterFee = _amount - _fee;
if (address(depositHook) != address(0)) {
  baseToken.approve(address(depositHook), _fee);
  // hook will calculate fee = _amount - _amountAfterFee
  depositHook.hook(_recipient, _amount, _amountAfterFee);
  baseToken.approve(address(depositHook), 0);
}

In the DepositHook, if there is a fee two actions take place:

1. Collect the fee to the treasury from Collateral contract
2. Send rewards relative to fee amount, to the depositor

uint256 _fee = _amountBeforeFee - _amountAfterFee;
if (_fee > 0) {
  collateral.getBaseToken().transferFrom(address(collateral), _treasury, _fee);
  _tokenSender.send(_sender, _fee);
}

The issue is that TokenSender has an early exit problem:

function send(address recipient, uint256 unconvertedAmount) external override onlyAllowedMsgSenders {
  uint256 scaledPrice = (_price.get() * _priceMultiplier) / MULTIPLIER_DENOMINATOR;
  if (scaledPrice <= _scaledPriceLowerBound) return;
  uint256 outputAmount = (unconvertedAmount * _outputTokenDecimalsFactor) / scaledPrice;
  if (outputAmount == 0) return;
  // EARLY EXIT PROBLEM
  if (outputAmount > _outputToken.balanceOf(address(this))) return;
  _outputToken.transfer(recipient, outputAmount);
}

In the code above, note that if the TokenSender does not currently have enough reward tokens to hand out, it will simply return successfully from the call. Therefore, user which assumed they will be getting cash back rewards from fees when depositing or withdrawing, are actually paying the fees with no compensation.

Impact

Permanent freeze of yield when TokenSender rewards bank is depleted and deposit or withdraw is called.

Proof of Concept

1. User has $1000 USDC
2. User deposits it to Collateral.sol, which charges 10% deposit fee
3. Meanwhile, TokenSender offers PPO rewards, but has just ran out.
4. DepositHook calls TokenSender.send(), which should send $100 fee rewards in PPO to user. send() returns without paying.
5. User is unhappy having assumed they will receive PPO from the fee.

Tools Used

Manual audit

Recommended Mitigation Steps

The root cause is that there is no differentiation between user’s request to mint expecting rewards, and user’s request to mint in any case. This lack of acknoledgement means user may be left under-compensated.

It is very advisable to add “forfeitRewards” flag, which is required to be true when TokenSender is not able to satisfy the reward owings to user.

Judging note

Permanent freezing of unclaimed yield is always rated as High severity on Immunefi bounties.

---

The text was updated successfully, but these errors were encountered:

All reactions