hats-finance / Convergence---Convex-integration-0xb3df23e155b74ad2b93777f58980d6727e8b40bb

0 stars 1 forks source link

CvgSvxStakingPositionService.sol - All calls to `curvePool` are wrong, as they use incorrect `i` and `j`. #16

Open hats-bug-reporter[bot] opened 6 months ago

hats-bug-reporter[bot] commented 6 months ago

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

Description: Description\ Let's examine _convertCvxToCvgCvx.

function _convertCvxToCvgCvx(
        uint256 cvxAmount,
        uint256 minCvgCvxAmountOut,
        bool isEthDeposit
    ) internal returns (uint256) {
        uint256 balance = cvxConvergenceLocker.balanceOf(address(this));
        if (!isEthDeposit) {
            /// @dev transfer CVX from user to this contract
            CVX.transferFrom(msg.sender, address(this), cvxAmount);
        }

        /// @dev Acts as a peg keeper and will prefers swap in the liquid pool in case of a depeg
        uint256 feesAmount = (cvxAmount * cvxConvergenceLocker.mintFees()) / 10_000;
        ICrvPoolPlain _curvePool = curvePool;
        if (_curvePool.get_dy(0, 1, cvxAmount) > ((cvxAmount - feesAmount) * depegPercentage) / HUNDRED) {
            require(minCvgCvxAmountOut >= cvxAmount, "INVALID_SLIPPAGE");

            /// @dev peg is too low, we swap in the LP with the CVX sent
            cvx1.mint(address(this), cvxAmount);
            _curvePool.exchange(0, 1, cvxAmount, minCvgCvxAmountOut, address(this));
        } else {
            /// @dev peg OK, we pass through the mint process 1:1
            cvxConvergenceLocker.mint(address(this), cvxAmount, false);
        }

        return cvxConvergenceLocker.balanceOf(address(this)) - balance;
    }

The function does 2 calls to curvePool, calling get_dy and potentially exchange.

When either of these 2 functions are called, the first two parameters specify tokenIn and tokenOut, represented with id's.

Since the pool is CvgCvx/CVX1, CvgCvx will be 0 and CVX1 will be 1.

Knowing this, the 2 calls are wrong, as for tokenIn we are using 0, meaning CvgCvx and for tokenOut we are using 1, meaning CVX1.

This is especially problematic in curvePool.exchange as we are supposed to swap CVX1 -> CvgCvx and we call:

_curvePool.exchange(0, 1, cvxAmount, minCvgCvxAmountOut, address(this));

In reality we are doing the oposite, as CvgCvx = 0 and CVX1 = 1, we are attempting to swap CvgCvx -> CVX1 since the first value is tokenIn and the second is tokenOut.

This is an issue all throught the code, wherever we are calling curvePool. Assuming that the comment above the variable is correct, this is an issue.

 /// @notice cvgCVX/CVX1 curve pool
    ICrvPoolPlain public curvePool;

Attachments

  1. Proof of Concept (PoC) File PoC in comments

  2. Revised Code File (Optional) If the pool will be cvgCvx/CVX1 then the indeces for the tokens will be cvgCvx = 0 and CVX1 = 1 and that's how they should be used in the protocol.

PlamenTSV commented 6 months ago

At first I got confused about this too, but it turns out the comments are wrong, the curve pool is correct: "Actually coin0 is CVX1 and coin1 is cvgCVX so the side is correct for the get_dy and the exchange (you can check the order of the coins in the convex-fixture at the deployment of the curve pool)"