sherlock-audit / 2022-10-illuminate-judging

3 stars 0 forks source link

hyh - Funds will be lost for Swivel lend() caller if it be run with another Yield Space pool and zero premiumSlippage #149

Closed sherlock-admin closed 2 years ago

sherlock-admin commented 2 years ago

hyh

medium

Funds will be lost for Swivel lend() caller if it be run with another Yield Space pool and zero premiumSlippage

Summary

Swivel version of lend() doesn't check the pool provided, y, to be the correct pool for the underlying and maturity. if a user mistakingly supplied non-malicious, but incorrect pool for lend() due to operational mistake (say provided otherwise correct pool, but corresponding to some another maturity), and set premiumSlippage to be zero, the PT tokens obtained from premium swap will be fully lost for the caller.

Vulnerability Detail

Swivel lend() will evaluate how much IMarketPlace(marketPlace).token(u, m, p) PTs was gained in total from Swivel orders executed and the selling of the premium, minting the same amount of Illuminate PTs to the caller.

If Yield Space pool provided does not correspond to the (u, m) combination, being otherwise correct, swivelLendPremium() will mint another type of PT, which will be unaccounted for Illuminate PT quantity calculation and not minted, i.e. it will be lost for the user.

Impact

If there is the PT in the system that y pool produced a redeem for it can be called thereafter and these funds will be socialized among the holders of the corresponding Illuminate PT shares.

If there is no such PT that the y pool produced in Illuninate (i.e. no such market was created via createMarket), these tokens will be permanently frozen on Lender's balance.

In both cases the funds are lost permanently for the user. However, setting the severity to be medium as the prerequisite is the misconfiguration of the parameters. Notice that its probability isn't low, as it is enough, for example, to use a pool of any wrong maturity, all other things being correct.

Code Snippet

Swivel lend() calls swivelLendPremium() to sell the underlying left after Swivel orders were filled:

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Lender.sol#L349-L449

    /// @notice lend method signature for Swivel
    /// @param p principal value according to the MarketPlace's Principals Enum
    /// @param u address of an underlying asset
    /// @param m maturity (timestamp) of the market
    /// @param a array of amounts of underlying tokens lent to each order in the orders array
    /// @param y Yield Space Pool for the Illuminate PT in this market
    /// @param o array of Swivel orders being filled
    /// @param s array of signatures for each order in the orders array
    /// @param e flag to indicate if returned funds should be swapped in Yield Space Pool
    /// @param premiumSlippage slippage limit, minimum amount to PTs to buy
    /// @return uint256 the amount of principal tokens lent out
    function lend(
        uint8 p,
        address u,
        uint256 m,
        uint256[] memory a,
        address y,
        Swivel.Order[] calldata o,
        Swivel.Components[] calldata s,
        bool e,
        uint256 premiumSlippage
    ) external unpaused(u, m, p) returns (uint256) {
        {
            // Check that the principal is Swivel
            ...

            // Lent represents the total amount of underlying to be lent
            uint256 lent = swivelAmount(a);

            // Transfer underlying token from user to Illuminate
            Safe.transferFrom(IERC20(u), msg.sender, address(this), lent);

            // Get the underlying balance prior to calling initiate
            uint256 starting = IERC20(u).balanceOf(address(this));

            // Verify and collect the fee
            ...

            uint256 received;
            {
                // Get the starting amount of principal tokens
                uint256 startingZcTokens = IERC20(
                    IMarketPlace(marketPlace).token(u, m, p)
                ).balanceOf(address(this));

                // Fill the given orders on Swivel
                ISwivel(swivelAddr).initiate(o, a, s);

                if (e) {
                    // Calculate the premium
                    uint256 premium = IERC20(u).balanceOf(address(this)) -
                        starting;

                    // Swap the premium for Illuminate principal tokens
                    swivelLendPremium(u, m, y, premium, premiumSlippage);
                }

                // Compute how many principal tokens were received
                received =
                    IERC20(IMarketPlace(marketPlace).token(u, m, p)).balanceOf(
                        address(this)
                    ) -
                    startingZcTokens;
            }

            // Mint Illuminate principal tokens to the user
            IERC5095(principalToken(u, m)).authMint(msg.sender, received);

            {
                emit Lend(
                    ...
                );
            }
            return received;
        }
    }

swivelLendPremium() calls yield():

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Lender.sol#L959-L979

    /// @notice lends the leftover underlying premium to the Illuminate PT's Yield Space Pool
    function swivelLendPremium(
        address u,
        uint256 m,
        address y,
        uint256 p,
        uint256 slippageTolerance
    ) internal {
        // Lend remaining funds to Illuminate's Yield Space Pool
        uint256 swapped = yield(
            u,
            y,
            p,
            address(this),
            IMarketPlace(marketPlace).token(u, m, 0),
            slippageTolerance
        );

        // Mint the remaining tokens
        IERC5095(principalToken(u, m)).authMint(msg.sender, swapped);
    }

yield() calls y to sell the underlying for PT:

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Lender.sol#L919-L957

    /// @notice swaps underlying premium via a Yield Space Pool
    /// @dev this method is only used by the Yield, Illuminate and Swivel protocols
    /// @param u address of an underlying asset
    /// @param y Yield Space Pool for the principal token
    /// @param a amount of underlying tokens to lend
    /// @param r the receiving address for PTs
    /// @param p the principal token in the Yield Space Pool
    /// @param m the minimum amount to purchase
    /// @return uint256 the amount of tokens sent to the Yield Space Pool
    function yield(
        address u,
        address y,
        uint256 a,
        address r,
        address p,
        uint256 m
    ) internal returns (uint256) {
        // Get the starting balance (to verify receipt of tokens)
        uint256 starting = IERC20(p).balanceOf(r);

        // Get the amount of tokens received for swapping underlying
        uint128 returned = IYield(y).sellBasePreview(Cast.u128(a));

        // Send the remaining amount to the Yield pool
        Safe.transfer(IERC20(u), y, a);

        // Lend out the remaining tokens in the Yield pool
        IYield(y).sellBase(r, returned);

        // Get the ending balance of principal tokens (must be at least starting + returned)
        uint256 received = IERC20(p).balanceOf(r) - starting;

        // Verify receipt of PTs from Yield Space Pool
        if (received <= m) {
            revert Exception(11, received, m, address(0), address(0));
        }

        return received;
    }

There are no checks for y correctness in this call sequence.

If y is a wrong pool and m is set to zero, the funds are fully lost for the caller as lend() will observe zero received and mint nothing to msg.sender for the premium part, while PTs of the y, that were obtained for the selling of a underlying amount, will remain on the Lender's balance.

If this type of PT be used elsewhere in the protocol then this amount can be redeemed thereafter, otherwise these funds will be permanently frozen as no redeem be called for that type of PT and it will remain left on Lender's balance unclaimed.

Tool used

Manual Review

Recommendation

Consider adding the check similarly to Yield version of lend() that also utilizes yield() call to sell underlying for PT:

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Lender.sol#L324-L328

            // Make sure the Yield Space Pool matches principal token
            address fyToken = IYield(y).fyToken();
            if (IYield(y).fyToken() != principal) {
                revert Exception(12, 0, 0, fyToken, principal);
            }
sourabhmarathe commented 2 years ago

We consider this within input validation and as a result, this would be considered beyond the scope of the audit.