Liquidation withdrawal fee is wrong, overcharging users by a factor of 1/(1-fee)

Summary

The fee math is incorrect for post-liquidation amounts, overcharging them by a multiplicative factor of 1/(1-fee)

Vulnerability Detail

The LockstakeEngine has the goal of charging a fee on all "exits", whether these happen via a free or a liquidation

In the ordinary case the fee is assessed as:

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/dba30d7a676c20dfed3bda8c52fd6702e2e85bb1/lockstake/src/LockstakeEngine.sol#L374-L377

uint256 burn = wad * fee_ / WAD;
        if (burn > 0) {
            mkr.burn(address(this), burn);
        }

However, in the case of liquidations the fee is computed as: https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/dba30d7a676c20dfed3bda8c52fd6702e2e85bb1/lockstake/src/LockstakeEngine.sol#L442-L444

        if (left > 0) {
            burn = _min(sold * fee / (WAD - fee), left);
            mkr.burn(address(this), burn);

this leads to over-charging the fee by a factor of 1/(WAD-fee)

Given that MakerDAO's governance settings intend to use double digit percentages the impact will result in a substantial additional fee taken, to the detriment of liquidated accounts causing them an additional loss of funds compared to the intended amount

Impact

The impact is effectively the same as taking an additional fee on the fee being assessed, these examples are based on MIP-101

Ranging from 15% to 40%:

We can see that at 15%, the math is taking an additional 17% of fee
And at 40% an additional 66.67% is being assessed

Fee	AMT	On Free	On Remove	Delta	%
1.50E+17	1.00E+18	150,000,000,000,000,000.00	176,470,588,235,294,000.00	2.65E+16	117.65
2.50E+17	1.00E+18	250,000,000,000,000,000.00	333,333,333,333,333,000.00	8.33E+16	133.33
3.00E+17	1.00E+18	300,000,000,000,000,000.00	428,571,428,571,429,000.00	1.29E+17	142.86
4.00E+17	1.00E+18	400,000,000,000,000,000.00	666,666,666,666,667,000.00	2.67E+17	166.67

Full formulas and chart are available here: https://docs.google.com/spreadsheets/d/1SzoBoI2PIO1lyFlVXXA0rg1YwK2HHKL4fMuFb-9N19U/edit?usp=sharing

Code Snippet

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/dba30d7a676c20dfed3bda8c52fd6702e2e85bb1/lockstake/src/LockstakeEngine.sol#L439-L443

Tool used

Manual Review

Recommendation

This line:

burn = _min(sold * fee / (WAD - fee), left);

Should be changed to

burn = _min(sold * fee / (WAD), left);

Duplicate of #20

sherlock-audit / 2024-06-makerdao-endgame-judging

GalloDaSballo - Liquidation withdrawal fee is wrong, overcharging users by a factor of 1/(1-fee) #98