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

1 stars 1 forks source link

Audittens - LockstakeEngine users can avoid paying exit fees #118

Closed sherlock-admin2 closed 1 month ago

sherlock-admin2 commented 1 month ago

Audittens

Medium

LockstakeEngine users can avoid paying exit fees

LockstakeEngine users can avoid paying exit fees

Summary

LockstakeEngine introduces exit fee for users who would like to withdraw their MKR. However, it's possible to withdraw all funds without paying any fees, by withdrawing funds in small portions.

Vulnerability Detail

The following code is used for calculation of the amount of exit fee in _free() function: uint256 burn = wad * fee_ / WAD;. Due to rounding down, the result of the calculation can be up to 1 wei of MKR smaller than the result of exact division. This allows splitting withdrawal into many small withdrawals, each of which will save 1 wei of MKR for user. For example, if exit fee is 15% (i.e. $fee_ = 0.15 \cdot 10^{18}$), a user can withdraw their funds in portions of 6 wei of MKR each, paying in total zero exit fee: $(6 \cdot 0.15 \cdot 10^{18}) / WAD = (0.9 \cdot 10^{18})/10^{18} = 0$

Impact

Not paying exit fees can be considered as stealing the corresponding amount of funds from the protocol, because not burning MKR results in not increasing the price of MKR. This leads to all MKR holders not gaining expected profit from burn operation.

As stated in the README, exit fee will typically be 15%. Combining this with the fact that the vulnerability can be exploited by any LockstakeEngine user, the total amount of funds affected is the same order of magnitude as the total amount of MKR. Therefore such vulnerability can potentially lead to up to 15% losses for the protocol which is classified as high impact.

Likelihood

Despite the fact that vulnerability can be exploited by any user, withdrawing all funds require a large number of transactions, which makes this vulnerability hard to exploit. Therefore likelihood of the issue is low.

Severity

Combining low likelihood and high impact together results in medium severity.

Code Snippet

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/lockstake/src/LockstakeEngine.sol#L373

Tool used

Manual Review

Recommendation

Change the rounding down into rounding up in the calculation of the fee amount:

- uint256 burn = wad * fee_ / WAD;
+ uint256 burn = _divup(wad * fee_, WAD);

This way partial withdraws become not profitable because each of them will always burn an amount of fee that is not less than the exact amount. Also, this recommendation doesn't decrease the user experience of any regular user, because it'll burn at most 1 extra wei of MKR per each free() operation.

sunbreak1211 commented 1 month ago

This is clearly explicited in the README of the repo: "Freeing very small amounts could bypass the exit fees (due to the rounding down) but since the LSE is meant to only be deployed on Ethereum, this is assumed to not be economically viable."