code-423n4 / 2021-04-marginswap-findings

1 stars 0 forks source link

maintainer can be pushed out #5

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Email address

mail@gpersoon.com

Handle

gpersoon

Eth address

gpersoon.eth

Vulnerability details

The function liquidate (in both CrossMarginLiquidation.sol and IsolatedMarginLiquidation.sol) can be called by everyone. If an attacker calls this repeatedly then the maintainer will be punished and eventually be reported as maintainerIsFailing And then the attacker can take the payouts

Proof of concept

When a non authorized address repeatedly calls liquidate then the following happens: isAuthorized = false which means maintenanceFailures[currentMaintainer] increases after sufficient calls it will be higher than the threshold and then maintainerIsFailing() will be true This results in canTakeNow being true which finally means the following will be executed: Fund(fund()).withdraw(PriceAware.peg, msg.sender, maintainerCut);

Impact

An attacker can push out a maintainer and take over the liquidation revenues

Tools used

remix

Recommended mitigation steps

put authorization on who can call the liquidate function review the maintainer punishment scheme

werg commented 3 years ago

I believe this issue is not a vulnerability, due to the checks in lines 326-335. Even if someone comes in first and claims the maintainer is failing they can do their job in the same or next block and get all / most of their failure record extinguished.

zscole commented 3 years ago

Acknowledging feedback from @werg, but maintaining the reported risk level of medium since this has implications on token logic.