sherlock-audit / 2022-10-illuminate-judging

3 stars 0 forks source link

Jeiwan - Re-entrancy in Sense redemption allows an attacker to inflate holdings and get more underlying tokens #217

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Jeiwan

high

Re-entrancy in Sense redemption allows an attacker to inflate holdings and get more underlying tokens

Summary

Re-entrancy in Sense redemption allows an attacker to inflate holdings and get more underlying tokens

Vulnerability Detail

The Sense's redeem function takes a Sense adapter address as the a parameter and calls it without proper validation (Redeemer.sol#L342-L398). This allows an attacker to supply the address of an exploit contract that calls the other redeem function with the same underlying and maturity. As a result, the increase of the underlying tokens will be counted twice: once in the other redeem function and once in the Sense's redeem function (Redeemer.sol#L325-L329, Redeemer.sol#L385-L394). The attacker will then be able to redeem their iPT tokens for more underlying tokens than expected (Redeemer.sol#L421-L431).

Impact

Consider this attack scenario:

  1. Suppose there are two markets with the same underlying tokens and maturity: Yield and Sense. And suppose that the Yield market has a bigger number of the principal tokens (i.e. users have lended more money to the Yield market).
  2. After maturity, an attacker calls the Sense redeem function and passes the address of its exploit contract as the a argument (Redeemer.sol#L347).
  3. The exploit contract implements these functions to skip the actual redeeming on Sense:
  4. During the redemption of the Yield market, the Yield external PT tokens will be redeemed for underlying tokens and holdings will be increased for the underlying token and maturity (Redeemer.sol#L325-L329).
  5. After the Yield market redemption, the rest of the Sense's redeem function will be executed: redeemed will be greater than amount because the Yield market was bigger; holdings will be increased again even though underlying token balance was increased only once (Redeemer.sol#L385-L397).
  6. The attacker will be able to redeem their iPT tokens right away to get more underlying tokens since the holdings mapping was inflated (Redeemer.sol#L422).
  7. Sense redemption won't be possible anymore since the amount will always be 0 (Redeemer.sol#L361) (0 tokens cannot be redeemed). The entire Lender's balance of the Sense principal tokens was moved in the previous call to the redeem function, which didn't trigger actual redemption because the exploit contract was called (Redeemer.sol#L364).

    Code Snippet

    Redeemer.sol#L342:

    function redeem(
    uint8 p,
    address u,
    uint256 m,
    uint256 s,
    address a
    ) external returns (bool) {
    // Check the principal is Sense
    if (p != uint8(MarketPlace.Principals.Sense)) {
        revert Exception(6, p, 0, address(0), address(0));
    }
    
    // Get Sense's principal token for this market
    IERC20 token = IERC20(IMarketPlace(marketPlace).token(u, m, p));
    
    // Cache the lender to save on SLOAD operations
    address cachedLender = lender;
    
    // Get the balance of tokens to be redeemed by the user
    uint256 amount = token.balanceOf(cachedLender);
    
    // Transfer the user's tokens to the redeem contract
    Safe.transferFrom(token, cachedLender, address(this), amount);
    
    // Get the starting balance to verify the amount received afterwards
    uint256 starting = IERC20(u).balanceOf(address(this));
    
    // Get the divider from the adapter
    ISenseDivider divider = ISenseDivider(ISenseAdapter(a).divider());
    
    // Redeem the tokens from the Sense contract
    ISenseDivider(divider).redeem(a, s, amount);
    
    // Get the compounding token that is redeemed by Sense
    address compounding = ISenseAdapter(a).target();
    
    // Redeem the compounding token back to the underlying
    IConverter(converter).convert(
        compounding,
        u,
        IERC20(compounding).balanceOf(address(this))
    );
    
    // Get the amount received
    uint256 redeemed = IERC20(u).balanceOf(address(this)) - starting;
    
    // Verify that underlying are received 1:1 - cannot trust the adapter
    if (redeemed < amount) {
        revert Exception(13, 0, 0, address(0), address(0));
    }
    
    // Update the holdings for this market
    holdings[u][m] = holdings[u][m] + redeemed;
    
    emit Redeem(p, u, m, redeemed, msg.sender);
    return true;
    }

    Tool used

    Manual Review

    Recommendation

    Consider validating the a argument of the Sense's redeem function, e.g. checking it against a list of valid adapters.

Duplicate of #236