code-423n4 / 2022-04-mimo-findings

0 stars 0 forks source link

The liquidator can double dip from the liquidation #134

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-mimo/blob/main/core/contracts/liquidityMining/v2/PARMinerV2.sol#L105-L130

Vulnerability details

Impact

To incentivize liquidations, callers are rewarded with _liquidateCallerReward. However, a greedy liquidator can take a bigger slice of the pie by sandwiching the liquidation. Liquidate is a public function with no protection against custom smart contracts. It takes an argument named dexTxData that is passed to the router which is supposed to sell the collateral token for the PAR token. There are no explicit parameters for slippage, thus I expect they are encoded in bytes of dexTxData. Because the liquidators do not care about the slippage, they can set min amounts to 0 or even worse, sandwich buy PAR, liquidate and then sell PAR for an increasing price thus making extra profit. Sandwiching might be a separate problem on its own because malicious actors can frontrun honest liquidators and make it a terrible experience.

Recommended Mitigation Steps

It is not an easy problem to solve. Consider a better approach to handling slippage in such cases, e.g. having a global tolerance level or initially starting with a whitelisted set of liquidators.

m19 commented 2 years ago

Technically possible, similar to #83. While this attack is possible and easier to execute than #83, liquidators can still use VaultsCore.liquidate directly and take all the profit for themselves.

gzeoneth commented 2 years ago

On second thought I believe this is not a duplicate of #83. While they share similar root cause, #83 provided a complete POC to steal the collateral + profit, while here only the profit is at risk. Downgrading to Low / QA.

gzeoneth commented 2 years ago

Consider with #137