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

1 stars 1 forks source link

chaduke - Wrong access control for LockstakeEngine.freeNoFee() opens the door for urn owners to avoid paying ```tax```. #46

Closed sherlock-admin2 closed 1 month ago

sherlock-admin2 commented 1 month ago

chaduke

High

Wrong access control for LockstakeEngine.freeNoFee() opens the door for urn owners to avoid paying tax.

Summary

LockstakeEngine.freeNoFee() should have the auth modifier to allow authorized access to enable promotion for free withdrawl. Howeve, the modifier is urnAuth(urn), therefore, the url owner can always call this function to free mkr from LockstakeEngine without paying any fee. There is no need to call other free functions, which will charge a fee.

I believe the intended design is to use auth instead of urnAuth(urn) as the modifier to enable promotion, etc. Now this function opens the door for urn owners to avoid paying "tax".

This attack can be replayed indefinitely, therefore, I mark this as HIGH

Root Cause

LockstakeEngine.freeNoFee() has the wrong modifier urnAuth(urn) instead of auth.

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

Internal pre-conditions

None.

External pre-conditions

None

Attack Path

The owner of an urn can simply call LockstakeEngine.freeNoFee() to free mkr without paying any fee. The owner will never call other free versions which needs to pay for a fee.

Impact

The system will loss fee for withdrawal always since nobody will call other versions of free functions. They will always call LockstakeEngine.freeNoFee() to avoid the fee.

PoC

An owner for a urn simply can call LockstakeEngine.freeNoFee() to avoid any fee.

Mitigation

Change modifier from urnAuth(urn) to auth.

telome commented 1 month ago

freeNoFee does have an auth modifier in addition to the urnAuth modifier.