Inaccurate tappFee and callerFee calculations

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/PrimaryLiquidationFacet.sol#L223

Vulnerability details

Impact

The _liquidationFeeHandler function calculation and distribution of the tappFee and callerFee is flawed. Specifically, there seems to be a potential issue with the logic in the else block, which is executed when TAPP.ethEscrowed is less than callerFee.

In the else block, the code attempts to give the caller a portion of the tappFee instead of the gasFee. However, the calculation appears to be incorrect.

Proof of Concept

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/PrimaryLiquidationFacet.sol#L223

VaultUser.ethEscrowed += callerFee - m.gasFee + tappFee;

This line is adding the callerFee, subtracting the gasFee, and then adding the tappFee to VaultUser.ethEscrowed. The issue is that the tappFee should not be added to the caller's escrow balance. The tappFee should be deducted from the TAPP escrow balance, as it is the fee meant for the TAPP contract.

Additionally, the line m.totalFee -= m.gasFee; in the else block seems unnecessary, as the gasFee is already accounted for in the callerFee calculation.

Tools Used

Manual

Recommended Mitigation Steps

} else {
    // Give caller the full callerFee since TAPP.ethEscrowed is insufficient
    VaultUser.ethEscrowed += callerFee;
    TAPP.ethEscrowed = 0;
}

In this version, the caller receives the full callerFee when TAPP.ethEscrowed is insufficient. The TAPP.ethEscrowed is set to 0, effectively transferring the remaining balance to the caller.

It's also worth double-checking the calculations and logic for the distribution of fees to ensure that the intended behavior is correctly implemented.

Assessed type

Math

[raymondfam]

Illogical/insufficient proof.

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[c4-judge]

hansfriese marked the issue as unsatisfactory: Insufficient proof