Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Mismatch of comment and value

0xakira

Vulnerability details

In the GmxProxy::getExecutionGasLimit() function when calculating executionGasLimit there is a comment that says to multiply by 1.2 to prevent transaction revert.

// multiply 1.2 (add some buffer) to ensure that the creation transaction does not revert.
executionGasLimit =
baseGasLimit +
((estimatedGasLimit + _callbackGasLimit) * multiplierFactor) /
PRECISION;

To do this, the multiplierFactor variable is used, which is taken from the DataStore contract using keccak256 as keccak256(abi.encode(‘ESTIMATED_GAS_FEE_MULTIPLIER_FACTOR’)). The DataStore multiplierFactor is taken from mapping(bytes32 => uint256) public uintValues. But instead of returning 1.2 it returns 1, which is inconsistent with the comment.

Impact

Now the transaction will be able to revert

Proof of Concept

function test_multiplierFactor() external {
address gmxProxy = address(PerpetualVault(vault).gmxProxy());
uint256 multiplierFactor = GmxProxy(payable(gmxProxy)).dataStore().getUint(GmxProxy(payable(gmxProxy)).ESTIMATED_GAS_FEE_MULTIPLIER_FACTOR());
assertEq(multiplierFactor, 1e30);
}
forge test --mt test_multiplierFactor --via-ir --rpc-url arbitrum -vv
​
Ran 1 test for test/PerpetualVault.t.sol:PerpetualVaultTest
[PASS] test_multiplierFactor() (gas: 38049)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 5.59ms (603.58µs CPU time)
​
Ran 1 test suite in 406.56ms (5.59ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation Steps

Consider changing the multiplierFactor from 1e30 to 1.2e30

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_multiplierFactor_no_buffer_returns_1_not_1.2

Likelihood: High, multiplierFactor always 1 and not 1.2 Impact: Informational/Very Low, there is no buffer for the transaction. However, this buffer does not seem to be necessary because the estimated gas limit, base gas limit, and callback gas limit are not underestimated. If it can be proven that, even with these variables being well-calculated (and no other root cause implied), the transaction will still revert, the impact could be higher.

Appeal created

n0kto Lead Judge
9 months ago
n0kto Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!