DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: low
Invalid

Fee Management Vulnerabilities

Summary

positionExecutionFee calculation uses tx.gasprice which could be manipulated

No upper bound on execution fees

Risk of ETH being locked if gas price spikes between fee calculation and execution

Vulnerability Details

function createOrder(
Order.OrderType orderType,
IGmxProxy.OrderData memory orderData
) public returns (bytes32) {
require(msg.sender == perpVault, "invalid caller");
uint256 positionExecutionFee = getExecutionGasLimit(
orderType,
orderData.callbackGasLimit
@>) * tx.gasprice;
require(
address(this).balance >= positionExecutionFee,
"insufficient eth balance"
);
// check if execution feature is enabled
bytes32 executeOrderFeatureKey = keccak256(
abi.encode(
EXECUTE_ORDER_FEATURE_DISABLED,
orderHandler,
orderType
)
);
function settle(
IGmxProxy.OrderData memory orderData
) external returns (bytes32) {
require(msg.sender == perpVault, "invalid caller");
uint256 positionExecutionFee = getExecutionGasLimit(
Order.OrderType.MarketDecrease,
orderData.callbackGasLimit
@>> ) * tx.gasprice;

Impact

positionExecutionFee can get manipulated. There is no gas limit on.

Tools Used

Recommendations

keep a limit on execution fees so that it cannot be manupliated.

Updates

Lead Judging Commences

n0kto Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

invalid_tx-gasprice_instable

The frontrunner won’t trigger "congestion" without a huge amount of transactions, and it will cost a lot. Moreover, the execution gas limit is overestimated to prevent such cases: ``` executionGasLimit = baseGasLimit + ((estimatedGasLimit + _callbackGasLimit) * multiplierFactor) / PRECISION; ``` The keeper won’t wait long to execute the order; otherwise, GMX would not be competitive.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.