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

11 stars 7 forks source link

blackhole - Keepers can steal additional execution fee from users in `processExecutionFee` function #245

Closed sherlock-admin3 closed 3 months ago

sherlock-admin3 commented 3 months ago

blackhole

Medium

Keepers can steal additional execution fee from users in processExecutionFee function

Summary

The processExecutionFee function is designed to compensate keepers for their execution work and refund any excess fees back to the user. However, due to the implementation not considering EIP-150, a malicious keeper can exploit this to drain all execution fees paid by users, irrespective of the actual execution costs.

Vulnerability Detail

All execution functions called by the keeper invoke the processExecutionFee function to handle fee payments. For example, the OrderFacet.executeOrder function is an external function subject to EIP-150, which restricts the gas available to sub-contracts. Specifically, only 63/64 of the gas is passed to the GasUtils sub-contract, while the remaining 1/64 is reserved and refunded to the keeper (msg.sender) after the transaction completes.

However, the calculation of gasUsed mistakenly includes this reserved gas portion. Since setting tx.gaslimit does not impact the actual gas cost of the transaction, the excess gas is refunded to the keeper. This allows the keeper to artificially inflate tx.gaslimit to manipulate the calculation of gasUsed. startGas

Impact

Malicious keepers can exploit this flaw to steal additional execution fees from users.

Code Snippet

https://github.com/sherlock-audit/2024-05-elfi-protocol/blob/main/elfi-perp-contracts/contracts/process/GasProcess.sol#L18

Tool used

Manual Review

Recommendation

To prevent keepers from exploiting the processExecutionFee function and stealing excess execution fees, adjust the usedGas calculation to ensure that only the actual gas cost is considered, excluding the reserved gas portion.

Reference

Duplicate of #95