Closed sherlock-admin closed 1 year ago
Escalate for 10 USDC
This report should be low/informational and duplicate of https://github.com/sherlock-audit/2023-05-perennial-judging/issues/123.
As already stated in the escalation of that report, liquidators get at least MIN_COLLATERAL*liquidationFee
unless the account holds less than that.
With the values 5% for liquidation fee and 10 USD for MIN_COLLATERAL, a malicious user would have to bring 10 USD of collateral and lose 95% of it for the liquidator to be affected, and have a chance to in turn affect the protocol for minuscule amounts.
This makes it a highly expensive operation, which does not qualify for medium impact on sherlock: https://docs.sherlock.xyz/audits/judging/judging
Escalate for 10 USDC
This report should be low/informational and duplicate of https://github.com/sherlock-audit/2023-05-perennial-judging/issues/123.
As already stated in the escalation of that report, liquidators get at least
MIN_COLLATERAL*liquidationFee
unless the account holds less than that. With the values 5% for liquidation fee and 10 USD for MIN_COLLATERAL, a malicious user would have to bring 10 USD of collateral and lose 95% of it for the liquidator to be affected, and have a chance to in turn affect the protocol for minuscule amounts.This makes it a highly expensive operation, which does not qualify for medium impact on sherlock: https://docs.sherlock.xyz/audits/judging/judging
You've created a valid escalation for 10 USDC!
To remove the escalation from consideration: Delete your comment.
You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.
(Posting similar comment on #103 and #123. These have the same impact, but arise from different mechanisms.)
I think that the escalation misses the problem that the issue points to.
The problem is not that liquidators will get less than MIN_COLLATERAL*liquidationFee
after 95% of the collateral has been liquidated.
The problem is that even after one liquidation, (as described in the report), the whole collateral of the account might be less than MIN_COLLATERAL
. Already at that point, liquidators have no incentive to liquidate the account.
So the comment in the escalation that the user would have to lose 95% of his collateral is wrong. Therefore I think that the escalation missed the problem and is invalid.
(Posting similar comment on #103 and #123. These have the same impact, but arise from different mechanisms.) I think that the escalation misses the problem that the issue points to. The problem is not that liquidators will get less than
MIN_COLLATERAL*liquidationFee
after 95% of the collateral has been liquidated. The problem is that even after one liquidation, (as described in the report), the whole collateral of the account might be less thanMIN_COLLATERAL
. Already at that point, liquidators have no incentive to liquidate the account.So the comment in the escalation that the user would have to lose 95% of his collateral is wrong. Therefore I think that the escalation missed the problem and is invalid.
Respectfully disagree, it seems that it is ok for a trader to have his collateral fall under MIN_COLLATERAL
(trivially since a trader can open a position with MIN_COLLATERAL, have losses being settled and such).
As I said in my escalation, the liquidator receives the same incentive whether the trader has MIN_COLLATERAL or MIN_COLLATERAL*liquidationFee
(5% of MIN_COLLATERAL). The liquidator begins to be less incentivized only if the collateral of the trader falls under 5% of MIN_COLLATERAL, as guaranteed by these lines:
https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial/contracts/collateral/Collateral.sol#L126-L129
Sorry, my mistake. I think you're right. Will investigate further later.
I agree with the escalation, issue should probably be changed to low.
While the issue shows that the user can open positions with less collateral than minCollateral
, as Sergei said, this initially won't matter for liquidators as they're guaranteed minCollateral*liquidationFee
.
The sponsor wrote in the similar issue #123:
This is a good callout - the minCollateral value should be set to give ample buffer to allow for safe liquidations even if the balance falls below this but it is a good case to note.
So indeed there should be ample buffer, but I now think that the method described here to go below minCollteral
is not enough of an issue (if at all) to be deemed a medium severity.
The liquidator begins to be less incentivized only if the collateral of the trader falls under 5% of MIN_COLLATERAL, as guaranteed by these lines:
I disagree with this. I used 5% maintenance (which is the minimum maintenance) as an example, but maintenance will most times be higher, and can even be up to 100%.
Unless @kbrizzle @arjun-io can make an argument for a reasonably high maintenance fee scenario, I would agree with @SergeKireev that the cost of the operation would be too high, and the value lost potentially too small.
This seems to be similar to #104 to me, in that the trader can attempt to create a position such that the liquidation is not incentivized although I don't think this is dependent on the product's maintenance value. In fact, a higher maintenance would increase the collateral available when liquidation occurs.
This seems to be similar to https://github.com/sherlock-audit/2023-05-perennial-judging/issues/104 to me, in that the trader can attempt to create a position such that the liquidation is not incentivized although I don't think this is dependent on the product's maintenance value. In fact, a higher maintenance would increase the collateral available when liquidation occurs.
Yes this is not related to maintenance, more to liquidationFee
(which is 5% on most already deployed markets).
However making this a duplicate of #104 would be a stretch, since this report only describes a procedure which allows a user to reduce her collateral below minCollateral
(misunderstanding the fact that the liquidator is not impacted right away). #104 demonstrates a stronger impact of systematically being able to reduce collateral to zero with a non-zero position for a cycle, and without great loss for the attacker
This issue explains how a user can cause his collateral balance to go below minCollateral by liquidating his account, and then open leveraged positions with it. When this happens, liquidators will not be incentivized to liquidate that account, thereby allowing the user to perform high-risk trades without fear of getting liquidated.
@SergeKireev Any response to @Emedudu's last comment?
I agree with the escalation, issue should probably be changed to low.
While the issue shows that the user can open positions with less collateral than
minCollateral
, as Sergei said, this initially won't matter for liquidators as they're guaranteedminCollateral*liquidationFee
. The sponsor wrote in the similar issue #123:This is a good callout - the minCollateral value should be set to give ample buffer to allow for safe liquidations even if the balance falls below this but it is a good case to note.
So indeed there should be ample buffer, but I now think that the method described here to go below
minCollteral
is not enough of an issue (if at all) to be deemed a medium severity.
@jacksanford1 latest @Emedudu comment does not add any new points which have not been addressed by @KenzoAgada comment quoted above
Liquidator is not always "guaranteed" minCollateral*liquidationFee
.
Fee that liquidator receives is min(totalCollateral, minCollateral*liquidationFee
). So there is a point at which liquidator will receive less than that(which would be disincentivizing).
To guide against this, Protocol does not allow user to withdraw his collateral to make it below minCollateral
.
So the problem is that user can cause his collateral to reach that point(where liquidators get dissuaded from liquidating that account) by liquidating his own account, and he won't lose anything doing that because the liquidation fee deducted from his collateral balance is paid to him.
@Emedudu this brings us back to the initial escalation, totalCollateral should be brought lower than minCollateral*liquidationFee
for the liquidator to see her rewards diminished:
With the values 5% for liquidation fee and 10 USD for MIN_COLLATERAL, a malicious user would have to bring 10 USD of collateral and lose 95% of it for the liquidator to be affected, and have a chance to in turn affect the protocol for minuscule amounts.
With current market parameters a malicious user would have to first lose 95% of her collateral in order to have a chance to impact the protocol
With current market parameters a malicious user would have to first lose 95% of her collateral in order to have a chance to impact the protocol
Firstly, the market parameters are not the same for all chains. On mainnet for example, liquidationFee is 10% and minCollateral is 100USD
Secondly, Malicious user won't "lose" 95% of his collateral because he will liquidate his account. So user is not disincentivized from doing this.
So the problem is that user can cause his collateral to reach that point(where liquidators get dissuaded from liquidating that account) by liquidating his own account, and he won't lose anything doing that because the liquidation fee deducted from his collateral balance is paid to him.
@Emedudu For the first point, if a reasonable person would assume the fee is in the 5-10% range, then I think that should be taken into account. If the vulnerability relies on an unrealistic liquidation fee (e.g. 90%), then I would lean against that argument.
@SergeKireev Does Emedudu have a point that they could liquidate their own account to mitigate this and thus make the vulnerability more economical?
@jacksanford1 @Emedudu
If we take the bigger liquidation fee from mainnet
: 10%
Alice cannot initially deposit less than minCollateral
To reach the desired state described in this report: totalCollateral == minCollateral*10%
by using liquidation:
Alice would need to be at minCollateral*20%
, which could be reached by self-liquidation from minCollateral*30%
and so on.
So we see that Alice could reach totalCollateral == minCollateral*10%
by repeatedly self-liquidating 9 times, and below that threshold a liquidator would be less incentivized to liquidate Alice's account
The example in the report suggests that the user should use market conditions to get liquidated, so in order to reach the desired state, the user would have to correctly predict market direction for 9 settlement periods but also take the risk that someone else liquidates the account and takes the liquidation reward (10% of collateral).
So to answer the question, yes it is possible to use self-liquidation. But a market participant which would be able to predict market direction 9 times in a row would profit more and take less risk by just taking the bets in the right direction.
This is the reason why I mentioned that user will use the max leverage possible to make his account easily liquidatable, so he does not need a lot of prediction. For example, if current price of market token is 1800USD, and user uses max leverage, and price moves by 1USD, user will be able to liquidate his account. User can also easily set up a bot that monitors his account, and liquidates it whenever it is liquidatable.
@Emedudu Do you agree with the premise that a user must correctly predict the market's directional movement many times (e.g. 9x) in a row?
The user does not have to "correctly predict market movement many times in a row".
As I said earlier, by using the max leverage possible, his position gets easily liquidatable. So he only needs the price of market token to flunctuate.
For example, in an ETH-long market with liquidationFee=10%
So, as we can see, it does not have to be in a row.
Using liquidationFee of 10% in ETH-long market, User needs price of eth on chainlink to record upward movement in eth price 9 times, and it does not have to be in a row. And for ETH-short market, User needs price of eth on chainlink to record downward movement in eth price 9 times. It also does not need to be in a row.
Looking at this chainlink price feed chart for ETH/USD on ethereum mainnet, you'll observe that between July 12 2023 at 9:55AM and July 13 2023 at 6:55 AM, there were 12 upward movements in ETH/USD price, and 14 downward movements in its price. So it is fair to say that there is ~50% chance of ETH price to move either up or down for each heartbeat. Hence, it is very feasible for a user to do this.
So User just needs to do these:
Additionally, User could also do this simultaneously on opposite markets(ETH-long and ETH-short) markets to double his chances.
@Emedudu With all due respect this argument is not quite valid, since each time the user settles positive PNL on his account he is 'set back' in this process.
In your example (it seems you meant ETH-short
instead of long, since user becomes liquidatable on prices rises but ok):
On third step:
Price moves down to 1700 USD- User opens new positions to make sure he is using max leverage
On this step the user has a massive positive PNL settled to his account which makes it way above minCollateral
again.
That's why the predictions have to be indeed consecutive.
Not to forget the user making his account publicly liquidatable takes a big risk of losing the liquidation fee to a more sophisticated actor (MEV) in the market. He has to take this risk >9 times in order to have a chance on making a small dent on the protocol.
You will agree with me @SergeKireev , that after doing the liquidations multiple times(e.g. 3 times), if price goes in an opposite direction, user won't have to start the liquidations process over again unless the PnL from that single price movement is >= minCollateral-totalCollateral. This is why it does not have to be consecutive(consecutive means if anything goes wrong once, user MUST start the whole process again).
Not to forget the user making his account publicly liquidatable takes a big risk of losing the liquidation fee to a more sophisticated actor (MEV) in the market.
User who is actively monitoring his account to liquidate it will surely beat other active liquidators who are monitoring many users' accounts at once. (Imagine the case where user has a bot that attempts to liquidate his account immediately after every chainlink price update for that market token).
after doing the liquidations multiple times(e.g. 3 times), if price goes in an opposite direction, user won't have to start the liquidations process over again unless the PnL from that single price movement is >= minCollateral-totalCollateral
Do you agree with that @SergeKireev?
User who is actively monitoring his account to liquidate it will surely beat other active liquidators who are monitoring many users' accounts at once.
I'd disagree with this. The effort required to monitor 20 accounts is likely similar to the effort required to monitor 1. Same for the likelihood of winning the liquidation.
after doing the liquidations multiple times(e.g. 3 times), if price goes in an opposite direction, user won't have to start the liquidations process over again unless the PnL from that single price movement is >= minCollateral-totalCollateral
Do you agree with that @SergeKireev?
By using current market parameters (e.g max leverage is 50x), an increase in price of 0.02% sets the user back one step in the process. Each time the user is set back by one step, he has to open his account one more time to public liquidation and assume additional risk
User who is actively monitoring his account to liquidate it will surely beat other active liquidators who are monitoring many users' accounts at once.
I'd disagree with this. The effort required to monitor 20 accounts is likely similar to the effort required to monitor 1. Same for the likelihood of winning the liquidation.
Exactly, as seen before, even if the MEV bots have only 1/9 ~ 11% of success rate against the individual monitoring his own account, this attack is not profitable
By using current market parameters (e.g max leverage is 50x), an increase in price of 0.02% sets the user back one step in the process.
An upward price movement sets the user one step back in the liquidations process, but it also means that user gained profit from the price movement(because his total collateral has increased). A downward price movement brings the user closer to his goal. So in either direction the price goes, user profits. Don't you think this is more incentive for the user to want to do this?
By using current market parameters (e.g max leverage is 50x), an increase in price of 0.02% sets the user back one step in the process.
An upward price movement sets the user one step back in the liquidations process, but it also means that user gained profit from the price movement(because his total collateral has increased). A downward price movement brings the user closer to his goal. So in either direction the price goes, user profits. Don't you think this is more incentive for the user to want to do this?
No this is not more incentive, since each time he opens his position to public liquidation he risks losing part of his collateral to a MEV bot. The setback means he has to open to liquidation one more time in the lengthy process
Normally, people open positions and hope that price always go in their favour so that they gain PnL. In the case of this user however, If price goes "in his favour", he gains PnL; if price does not go in his favour, he has a good chance of performing self liquidation because he is closely monitoring his account; and if he loses, his totalCollateral balance is closer to his desired state (minCollateral*liquidationFee). So the user gains in any direction price moves.
I see that there are some nuances here. For example @Emedudu's point:
user won't have to start the liquidations process over again unless the PnL from that single price movement is >= minCollateral-totalCollateral
means that maybe a lower number of consecutive downward price moves would be necessary as long as the upward price movement is not larger than the total downward ones. But I think there has been enough discussion around this issue to determine that the circumstances for this attack are very difficult to pull off and the reward is simply preventing a single account from being liquidated which is already a questionable impact imo.
Will mark this issue as Low unless there are stronger arguments presented.
But want to make sure the latest comments are visible to @arjun-io and the protocol team so that they are still comfortable with their approach to mitigating (or not mitigating) this.
Result: Low Has duplicates See message above for final comment on severity of issue.
Emmanuel
medium
Liquidating an account may cause collateral balance of the account to go below
minCollateral
Summary
There is a minimum collateral that is set in the Controller contract. This disallows users from depositing collateral that would make collateral balance less than minimum Collateral, and prevents withdraws that would make the collateral balance less than minimum collateral. This issue explains how an account can cause his collateral balance to go below the minCollateral and even open positions with it, hence defeating the purpose of minCollateral
Vulnerability Detail
Here is how the protocol calculates the liquidation fee that would be deducted from the account.
The
fee
is the minimum oftotalCollateral
andcollateralForFee.mul(liquidationFee)
which means that if fee is nottotalCollateral
, there is a possibility that the fee could be large enough to maketotalCollateral
fall belowminCollateral
Consider this scenario:
One of the purposes of minCollateral is to allow for gas costs that would cover liquidation.
NOTE: There is no strict mechanism to liquidate account when it falls below minCollateral
Impact
A user can cause his collateral balance to be any value, even below minCollateral. With this power, he can make his collateral balance such that there would be no incentive to liquidate it. Now, even when massive changes in Product's token price causes his collateral balance to be far less than the maintenance required, User's account may not be liquidated as there are no incentives.
Code Snippet
https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial/contracts/collateral/Collateral.sol#L126-L132
Tool used
Manual Review
Recommendation
Create a mapping called "unused collateral" When liquidating an account, if deducting liquidation fee causes account's balance to be less than minCollateral, increment the account's "unused collateral" balance by the account's balance, and change the
OptimisticLedger
balance of that account to 0. The "unused collateral" balances should not be used in any internal accounting, and should be claimable by the user at anytime.