Loss Fee does not get added due to wrong calculation

Summary

When the ROLE_KEEPER executes a transaction, we check if the execution_fee is sufficient to cover the transaction cost. If any amount remains, it is refunded to the user, and if the fee is insufficient, the excess amount is added to the loss amount. However, in the event of a loss, the loss amount is not correctly calculated and therefore not added due to an error in the calculation

Vulnerability Detail

Whenever user submit a request for any operation we charge executionFee in advance . The ROLE_KEPPER will submit the request operation and will charge the exectuion_fee. Here one pf the following 2 cases can occur.

The execution Fee was sufficient and the remaining amount sent back to users.

The executionFee was insufficient and loss added to Protocol. In 2nd case there is an Issue due to which the Loss will never be added.

function processExecutionFee(PayExecutionFeeParams memory cache) external {
    uint256 usedGas = cache.startGas - gasleft();
    uint256 executionFee = usedGas * tx.gasprice;
    uint256 refundFee;
    uint256 lossFee;
    if (executionFee > cache.userExecutionFee) {
@>            executionFee = cache.userExecutionFee;  
@>           lossFee = executionFee - cache.userExecutionFee;

    } else {
        refundFee = cache.userExecutionFee - executionFee;
    }
    VaultProcess.transferOut(
        cache.from,
        AppConfig.getChainConfig().wrapperToken,
        address(this),
        cache.userExecutionFee
    );
    VaultProcess.withdrawEther(cache.keeper, executionFee);
    if (refundFee > 0) {
        VaultProcess.withdrawEther(cache.account, refundFee);
    }
    if (lossFee > 0) {
        CommonData.addLossExecutionFee(lossFee);
    }
}

From above code We can observed that when executionFee>cache.userExecutionFee then we first assign executionFee = cache.userExecutionFee and then calculate LossFee , So LossFee will always be zero.

executionFee = 10 gwei;
cache.userExecutionFee = 9 gwei;
// we first assign 
executionFee = cache.userExecutionFee; // so Here executionFee = 9 gwei
lossFee = executionFee - cache.userExecutionFee;// 9 gwei - 9 gwei =0
    if (lossFee > 0) { // here lossFee=0 so it will not be added
        CommonData.addLossExecutionFee(lossFee);
    }

Impact

processExecutionFee will never added any loss occur.

Code Snippet

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

Tool used

Manual Review

Recommendation

diff --git a/elfi-perp-contracts/contracts/process/GasProcess.sol b/elfi-perp-contracts/contracts/process/GasProcess.sol
index 9a112074..b0552f34 100644
--- a/elfi-perp-contracts/contracts/process/GasProcess.sol
+++ b/elfi-perp-contracts/contracts/process/GasProcess.sol
@@ -20,8 +20,8 @@ library GasProcess {
         uint256 refundFee;
         uint256 lossFee;
         if (executionFee > cache.userExecutionFee) {
-            executionFee = cache.userExecutionFee;
             lossFee = executionFee - cache.userExecutionFee;
+            executionFee = cache.userExecutionFee;
         } else {
             refundFee = cache.userExecutionFee - executionFee;

Duplicate of #108

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

aman - Loss Fee does not get added due to wrong calculation #253