Order Creation with Zero Margin Due to Incorrect Execution Fee Validation

Summary

The vulnerability lies in the _validateGasFeeLimitAndInitialMargin function. If a user submits an order with an orderMargin equal to the executionFee, the function allows the creation of an order with zero margin. This bypasses checks that should prevent orders with zero margin from being submitted, potentially leading to a situation where invalid orders are processed.

Vulnerability Detail

The condition inside _validateGasFeeLimitAndInitialMargin allows for a situation where the margin is reduced by the execution fee, potentially to zero:

if (
    params.isNativeToken &&
    params.posSide == Order.PositionSide.INCREASE &&
    !params.isCrossMargin &&
    params.orderMargin >= params.executionFee
) {
    return (params.orderMargin - params.executionFee, true);
}

If params.orderMargin is equal to params.executionFee, the function returns zero for the order margin:

return (params.orderMargin - params.executionFee, true);

Impact

Orders with zero margin can be created breaking an invariant of the system.

Code Snippet

https://github.com/sherlock-audit/2024-05-elfi-protocol/blob/8a1a01804a7de7f73a04d794bf6b8104528681ad/elfi-perp-contracts/contracts/process/OrderProcess.sol#L453-L481

https://github.com/sherlock-audit/2024-05-elfi-protocol/blob/8a1a01804a7de7f73a04d794bf6b8104528681ad/elfi-perp-contracts/contracts/process/OrderProcess.sol#L47-L100

Tool used

Manual Review

Recommendation

Make sure that orderMargin is not equal to executionFee

sherlock-admin2 commented 2 weeks ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/0xCedar/elfi-perp-contracts/pull/39

nevillehuang commented 1 week ago

request poc

What impact does this have that meets sherlocks criteria for medium severity (fund loss/time sensitive DoS/break core contract functionality), it is not clearly described. I am inclined to set low severiy

sherlock-admin4 commented 1 week ago

PoC requested from @creat3xai

Requests remaining: 10

creat3xai commented 1 week ago

Hello @nevillehuang, One of the issues that this may cause is that it will lead to a user creating orders and increasing their position size without adjusting their margin, which may result in them being liquidated. Additionally, it may lead to the protocol losing funds if the price moves faster than expected.