sherlock-audit / 2022-10-illuminate-judging

3 stars 0 forks source link

ctf_sec - Redeemed amount in Redeemer.sol#authRedeem may be truncated #148

Closed sherlock-admin closed 2 years ago

sherlock-admin commented 2 years ago

ctf_sec

medium

Redeemed amount in Redeemer.sol#authRedeem may be truncated

Summary

Redeemed amount in Redeemer.sol#authRedeem may be truncated

Vulnerability Detail

Let us look into the implementation for Redeemer.sol#authRedeem

  // Get the principal token for the given market
  IERC5095 pt = IERC5095(IMarketPlace(marketPlace).token(u, m, 0));

  // Make sure the market has matured
  uint256 maturity = pt.maturity();
  if (block.timestamp < maturity) {
      revert Exception(7, maturity, 0, address(0), address(0));
  }

  // Calculate the amount redeemed
  uint256 redeemed = (a * holdings[u][m]) / pt.totalSupply();

  // Update holdings of underlying
  holdings[u][m] = holdings[u][m] - redeemed;

  // Burn the user's principal tokens
  pt.authBurn(f, a);

  // Transfer the original underlying token back to the user
  Safe.transfer(IERC20(u), t, redeemed);

note the line:

  // Calculate the amount redeemed
  uint256 redeemed = (a * holdings[u][m]) / pt.totalSupply();

Clearly the redeemed can be truncated, if the holdings[u][m] goes down, or pt.totalSupply() goes up.

Impact

redeemed can be truncated, if the holdings[u][m] goes down, or pt.totalSupply() goes up

even to 0, the redeemer find that the principle token is burned but redeem for nothing.

same truncation affects the overloaded authRedeem that has for loop in Redeemer.sol

// Loop through the provided arrays and mature each individual position
for (uint256 i; i != length; ) {
    // Fetch the allowance set by the holder of the principal tokens
    uint256 allowance = uToken.allowance(f[i], address(this));

    // Get the amount of tokens held by the owner
    uint256 amount = pt.balanceOf(f[i]);

    // Calculate how many tokens the user should receive
    uint256 redeemed = (amount * holdings[u][m]) / pt.totalSupply();

Code Snippet

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L454-L478

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L507-L529

Tool used

Manual Review

Recommendation

Add the slippage check, let user input a parameter, saying, this is the minimum amount of token I want to redeem if we redeem less than this amount, revert transaction, and please revert on 0 amount!

uint256 redeemed = (a * holdings[u][m]) / pt.totalSupply();
if(redeemed == 0) revert InvalidAmount();
sourabhmarathe commented 2 years ago

We consider this to be input validation (user should check holdings[u][m] prior to making call). We believe this can be done more effectively from outside of the protocol.