hats-finance / Thorn-protocol-0x1286ecdac50215a366458a14968fbca4bd95067d

GNU General Public License v3.0
0 stars 0 forks source link

Possible Out-of-gas DOS when doing multiple swaps via StableSwapRouter::exactInputStableSwap due to lengthy loop at StableSwapTwoPool::get_D() #88

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xccffd617c1771bb7f3121ec2e247ac82f0c553001270a64cd3d7d726bcbc5d22 Severity: high

Description: Description\ During times of high gas prices, protocol users are likely to experience many failed transactions while executing complex swaps due 2 potential expensive loops at StableSwapTwoPool::exactInputStableSwap.

Attack Scenario\ While doing a swap at StableSwapTwoPool::exchange, there are 2 potentially gas-expensive loops, i.e. StableSwapTwoPool::get_y() at Ln 367:

function get_y(
        uint256 i,
        uint256 j,
        uint256 x,
        uint256[N_COINS] memory xp_
    ) internal view returns (uint256) {
        // x in the input is converted to the same price/precision
        require((i != j) && (i < N_COINS) && (j < N_COINS), "Illegal parameter");
        uint256 amp = get_A();
        uint256 D = get_D(xp_, amp);
        uint256 c = D;
        uint256 S_;
        uint256 Ann = amp * N_COINS;

        uint256 _x;
        for (uint256 k = 0; k < N_COINS; k++) {
            if (k == i) {
                _x = x;
            } else if (k != j) {
                _x = xp_[k];
            } else {
                continue;
            }
            S_ += _x;
            c = (c * D) / (_x * N_COINS);
        }
        c = (c * D) / (Ann * N_COINS);
        uint256 b = S_ + D / Ann; // - D
        uint256 y_prev;
        uint256 y = D;

        for (uint256 m = 0; m < 255; m++) {
            y_prev = y;
            y = (y * y + c) / (2 * y + b - D);
            // Equality with the precision of 1
            if (y > y_prev) {
                if (y - y_prev <= 1) {
                    break;
                }
            } else {
                if (y_prev - y <= 1) {
                    break;
                }
            }
        }
        return y;
    }

and StableSwapTwoPool::get_D at Ln 245:

function get_D(uint256[N_COINS] memory xp, uint256 amp) internal pure returns (uint256) {
        uint256 S;
        for (uint256 i = 0; i < N_COINS; i++) {
            S += xp[i];
        }
        if (S == 0) {
            return 0;
        }

        uint256 Dprev;
        uint256 D = S;
        uint256 Ann = amp * N_COINS;
        for (uint256 j = 0; j < 255; j++) {
            uint256 D_P = D;
            for (uint256 k = 0; k < N_COINS; k++) {
                D_P = (D_P * D) / (xp[k] * N_COINS); // If division by 0, this will be borked: only withdrawal will work. And that is good
            }
            Dprev = D;
            D = ((Ann * S + D_P * N_COINS) * D) / ((Ann - 1) * D + (N_COINS + 1) * D_P);
            // Equality with the precision of 1
            if (D > Dprev) {
                if (D - Dprev <= 1) {
                    break;
                }
            } else {
                if (Dprev - D <= 1) {
                    break;
                }
            }
        }
        return D;
    }

These loops can cost users high gas prices that can exceed the block-gas limit leading to failed transactions especially during periods of high volatility. If an error occurs during 1 of the swaps, the whole transaction/trade will fail. Attachments

  1. Proof of Concept (PoC) File Consider a high volume trader wishing to swaps many times, maybe 20 tokens in 1 transaction:

    it("should consume much gas", async () => {

        let x = 0;
        while (x < 20) {
            await stableSwapRouter.exactInputStableSwap([ROSE,stROSE.address], [2],ADD_AMOUNT_1, 0,accounts[0].address,{ value: ADD_AMOUNT_1});
            x++;
        }

    });
  1. Revised Code File (Optional) Consider wrapping each individual swap in a try-catch block at StableSwapRouter::_swap :
function _swap(address[] memory path, uint256[] memory flag) private {
        uint256 amountIn_;
        require(path.length - 1 == flag.length);
        for (uint256 i; i < flag.length; i++) {
            try{
                (address input, address output) = (path[i], path[i + 1]);
                (uint256 k, uint256 j, address swapContract) = SmartRouterHelper
                    .getStableInfo(stableSwapFactory, input, output, flag[i]);
                if (input == ROSE) {
                    amountIn_ = address(this).balance;
                    IStableSwap(swapContract).exchange{value: amountIn_}(k, j, amountIn_, 0);
                }
                if (input != ROSE) {
                    amountIn_ = IERC20(input).balanceOf(address(this));
                    TransferHelper.safeApprove(input, swapContract, amountIn_);
                    IStableSwap(swapContract).exchange(k, j, amountIn_, 0);
                }

            }catch Error(string memory /*reason*/){

            }
        }
Ngugs commented 1 month ago

Kindly comment why the finding is invalid...Thanks