Wrong gas fee calculation

Summary

The executeOrder.executeOrder and executeOrder.cancelOrder run by ROLE_KEEPER will calculate the gas fee cost and return the remaining execution fee to the user. However, the calculated gas fee only covers part of the actual cost.

Vulnerability Detail

For example, the executeOrder.executeOrder function calculates the gas cost by subtracting the left gas at the start of processExecutionFee from the initial gas. However, the gas cost during the execution of processExecutionFee is not included.

    function executeOrder(
        uint256 orderId,
        OracleProcess.OracleParam[] calldata oracles
    ) external override {
        uint256 startGas = gasleft();
        RoleAccessControl.checkRole(RoleAccessControl.ROLE_KEEPER);
        Order.OrderInfo memory order = Order.get(orderId);
        if (order.account == address(0)) {
            revert Errors.OrderNotExists(orderId);
        }
        OracleProcess.setOraclePrice(oracles);
        OrderProcess.executeOrder(orderId, order);
        OracleProcess.clearOraclePrice();
        GasProcess.processExecutionFee(
            GasProcess.PayExecutionFeeParams(
                order.isExecutionFeeFromTradeVault
                    ? IVault(address(this)).getTradeVaultAddress()
                    : IVault(address(this)).getPortfolioVaultAddress(),
                order.executionFee,
                startGas,
                msg.sender,
                order.account
            )
        );
    }
    function processExecutionFee(PayExecutionFeeParams memory cache) external {
        /// @dev Calculates the gas used
        uint256 usedGas = cache.startGas - gasleft();
        /// @dev Calculates the execution fee based on gas used and gas price
        uint256 executionFee = usedGas * tx.gasprice;
        uint256 refundFee;
        uint256 lossFee;

        if (executionFee > cache.userExecutionFee) {
            executionFee = cache.userExecutionFee;
            lossFee = executionFee - cache.userExecutionFee;
        } else {
            /// @dev Calculates the refund fee if execution fee is less than user's fee
            refundFee = cache.userExecutionFee - executionFee;
        }

        /// @dev Transfers the user's execution fee to the contract
        VaultProcess.transferOut(
            cache.from,
            AppConfig.getChainConfig().wrapperToken,
            address(this),
            cache.userExecutionFee
        );

        /// @dev Withdraws the execution fee to the keeper
        VaultProcess.withdrawEther(cache.keeper, executionFee);

        if (refundFee > 0) {
            /// @dev Refunds the remaining fee to the user's account
            VaultProcess.withdrawEther(cache.account, refundFee);
        }

        if (lossFee > 0) {
            /// @dev Records the loss fee
            CommonData.addLossExecutionFee(lossFee);
        }
    }

Impact

The keeper will lose some profit due to the lower calculated gas fee.

Code Snippet

Tool used

Manual Review

Recommendation

Precalculate the gas cost needed by the processExecutionFee function and add it to the executionFee.

Duplicate of #147

sherlock-audit / 2024-05-elfi-protocol-judging

link - Wrong gas fee calculation #288