Swapping underlying for yield tokens is susceptible to precision loss

Summary

The swapUnderlyingForYt function in NapierRouter.sol swaps underlying asset for Yield Tokens (Yt). However, the function performs division before multiplication when calculating the uDeposit, which is the amount of underlying asset that needs to be deposited to issue Yt. This can lead to precision loss where users do not receive enough Yt than what they should.

Vulnerability Detail

To minimize loss of precision you should always do multiplications before division, since Solidity just rounds down when there is a remainder in the division operation. When calculating uDesposit, which is used to determine the amount of underlying asset that needs to be deposit, the following operation is performed:

    uint256 uDepositNoFee = cscale * ytOutDesired / maxscale;
    uDeposit = uDepositNoFee * MAX_BPS / (MAX_BPS - (series.issuanceFee + 1));

Here, uDespoitNoFee is divided by maxscale but then multiplied by MAX_BPS on the next line, allowing for the possibility of precision loss.

Impact

The actual amount of underlying asset calculated to be deposited might be less than what is required. This could result in a smaller amount of Yield Tokens (YT) being issued than desired.

Code Snippet

NapierRouter:swapUnderlyingForYt (#L286-385)

    function swapUnderlyingForYt(
        address pool,
        uint256 index,
        uint256 ytOutDesired,
        uint256 underlyingInMax,
        address recipient,
        uint256 deadline
    ) external payable override nonReentrant checkDeadline(deadline) returns (uint256) {
        // dev: Optimistically call to the `pool` provided by the untrusted caller.
        // And then verify the pool using CREATE2.
        (address underlying, address basePool) = INapierPool(pool).getAssets();

        // if `pool` doesn't matched, it would be reverted.
        if (INapierPool(pool) != PoolAddress.computeAddress(basePool, underlying, POOL_CREATION_HASH, address(factory)))
        {
            revert Errors.RouterPoolNotFound();
        }

        ITranche pt = ITranche(address(INapierPool(pool).principalTokens()[index]));

        uint256 uDeposit; // underlying asset to be deposited to Tranche
        {
            // This section of code aims to calculate the amount of underlying asset (`uDeposit`) required to issue a specific amount of PT and YT (`ytOutDesired`).
            // The calculations are based on the formula used in the `Tranche.issue` function.

            ITranche.Series memory series = pt.getSeries();

            // Update maxscale if current scale is greater than maxscale
            uint256 maxscale = series.maxscale;
            uint256 cscale = IBaseAdapter(series.adapter).scale();
            if (cscale > maxscale) {
                maxscale = cscale;
            }
            // Variable Definitions:
            // - `uDeposit`: The amount of underlying asset that needs to be deposited to issue PT and YT.
            // - `ytOutDesired`: The desired amount of PT and YT to be issued.
            // - `cscale`: Current scale of the Tranche.
            // - `maxscale`: Maximum scale of the Tranche (denoted as 'S' in the formula).
            // - `issuanceFee`: Issuance fee in basis points. (10000 =100%).

            // Formula for `Tranche.issue`:
            // ```
            // shares = uDeposit / s
            // fee = shares * issuanceFeeBps / 10000
            // pyIssue = (shares - fee) * S
            // ```

            // Solving for `uDeposit`:
            // ```
            // uDeposit = (pyIssue * s / S) / (1 - issuanceFeeBps / 10000)
            // ```
            // Hack:
            // Buffer is added to the denominator.
            // This ensures that at least `ytOutDesired` amount of PT and YT are issued.
            // If maximum scale and current scale are significantly different or `ytOutDesired` is small, the function might fail.
            // Without this buffer, any rounding errors that reduce the issued PT and YT could lead to an insufficient amount of PT to be repaid to the pool.
@>      uint256 uDepositNoFee = cscale * ytOutDesired / maxscale;
@>      uDeposit = uDepositNoFee * MAX_BPS / (MAX_BPS - (series.issuanceFee + 1)); // 0.01 bps buffer
        }

        // Abi encode callback data to be used in swapCallback
        bytes memory data = new bytes(0x120);
        {
            uint256 callbackType = uint256(CallbackType.SwapUnderlyingForYt);
            address yt = pt.yieldToken();
            assembly {
                // Equivanlent to:
                // abi.encode(CallbackType.SwapUnderlyingForYt, underlying, basePool, CallbackDataTypes.SwapUnderlyingForYtData({pt: pt, yt: yt, payer: msg.sender, recipient: recipient, underlyingDeposit: uDeposit, maxUnderlyingPull: underlyingInMax}))
                mstore(add(data, 0x20), callbackType)
                mstore(add(data, 0x40), underlying)
                mstore(add(data, 0x60), basePool)
                mstore(add(data, 0x80), caller()) // dev: Ensure 'payer' is always 'msg.sender' to prevent allowance theft on callback.
                mstore(add(data, 0xa0), pt)
                mstore(add(data, 0xc0), yt)
                mstore(add(data, 0xe0), recipient)
                mstore(add(data, 0x100), uDeposit)
                mstore(add(data, 0x120), underlyingInMax)
            }
        }
        uint256 received = INapierPool(pool).swapPtForUnderlying(
            index,
            ytOutDesired, // ptInDesired
            address(this), // this contract will receive underlying token from pool
            data
        );

        // Underlying pulled = underlying deposited - underlying received from swap
        return uDeposit - received;
    }

Tool used

Manual Review

Recommendation

Perform multiplication before division

sherlock-audit / 2024-01-napier-judging

crypticdefense - Swapping underlying for yield tokens is susceptible to precision loss #35