re-al-Foundation / rwa-contracts

0 stars 0 forks source link

[RHR-02M] Insecure Trade Output Measurement #64

Closed chasebrownn closed 4 months ago

chasebrownn commented 4 months ago

RHR-02M: Insecure Trade Output Measurement

Type Severity Location
Logical Fault RoyaltyHandler.sol:L286

Description:

The referenced statement will utilize the IQuoterV2 implementation to query the output of a particular swap, however, this mechanism is insecure as it will simply simulate the trade in on-chain conditions and thus not protect against slippage.

Impact:

The amount of tokens swapped in the RoyaltyHandler::_swapTokensForETH function is susceptible to arbitrage as it uses an insecure output measurement mechanism.

Example:

/**
 * @notice This internal method takes `tokenAmount` of tokens and swaps it for ETH.
 * @param tokenAmount Amount of $RWA tokens being swapped/sold for ETH.
 */
function _swapTokensForETH(uint256 tokenAmount) internal {

    // a. Get quote
    IQuoterV2.QuoteExactInputSingleParams memory quoteParams = IQuoterV2.QuoteExactInputSingleParams({
        tokenIn: address(rwaToken),
        tokenOut: address(WETH),
        amountIn: tokenAmount,
        fee: poolFee,
        sqrtPriceLimitX96: 0
    });
    (uint256 amountOut,,,) = quoter.quoteExactInputSingle(quoteParams);

    // b. build swap params
    ISwapRouter.ExactInputSingleParams memory swapParams = ISwapRouter.ExactInputSingleParams({
        tokenIn: address(rwaToken),
        tokenOut: address(WETH),
        fee: poolFee,
        recipient: address(this),
        deadline: block.timestamp,
        amountIn: tokenAmount,
        amountOutMinimum: amountOut,
        sqrtPriceLimitX96: 0
    });

    // c. swap
    rwaToken.approve(address(swapRouter), tokenAmount);

    uint256 preBal = WETH.balanceOf(address(this));
    swapRouter.exactInputSingle(swapParams);

    require(preBal + amountOut == WETH.balanceOf(address(this)), "RoyaltyHandler: Insufficient WETH received");
}

Recommendation:

We advise the code to utilize a user-provided minimum output value, or to utilize a decentralized price measurement mechanism (i.e. TWAP / Oracle) to ensure that the trade performed by the RoyaltyHandler::_swapTokensForETH function is secure against arbitrage attacks.

chasebrownn commented 4 months ago

Note: Research other methods to fetching quote without using Quoter

chasebrownn commented 4 months ago

Acknowledged