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

11 stars 7 forks source link

Unnecessary check in decrease position #292

Closed sherlock-admin2 closed 4 months ago

sherlock-admin2 commented 4 months ago

Unnecessary check in decrease position

Low/Info issue submitted by mstpr-brainbot

Summary

Unnecessary check in isolated position.

Vulnerability Detail

Isolated positions can be liquidated if the following condition has to be correct as its defined in DecreasePositionProcess::decreasePosition() function:

if (cache.settledMargin < 0 && !cache.isLiquidation && !position.isCrossMargin) {
            revert Errors.PositionShouldBeLiquidation();
        }

cache variable is filled in the DecreasePositionProcess::_updateDecreasePosition() internal function

As we can observe in the _updateDecreasePosition internal function cache.isLiquidation value is never assigned which means it is default to false.

no need to check it since its default to "false".

Impact

Code Snippet

https://github.com/sherlock-audit/2024-05-elfi-protocol/blob/8a1a01804a7de7f73a04d794bf6b8104528681ad/elfi-perp-contracts/contracts/process/DecreasePositionProcess.sol#L60-L77

https://github.com/sherlock-audit/2024-05-elfi-protocol/blob/8a1a01804a7de7f73a04d794bf6b8104528681ad/elfi-perp-contracts/contracts/process/DecreasePositionProcess.sol#L206-L336

Tool used

Manual Review

Recommendation

Remove the !cache.isLiquidation