sherlock-audit / 2023-04-hubble-exchange-judging

7 stars 6 forks source link

0x52 - Rogue validators can manipulate funding rates and profit unfairly from liquidations #183

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

0x52

high

Rogue validators can manipulate funding rates and profit unfairly from liquidations

Summary

Validators are given the exclusive privilege to match. Any validator can abuse this privilege to manipulate funding rates and profit unfairly from liquidations. Normally validators are fully trusted but in the circumstances of these smart contracts and the chain it's deployed on, practically anyone can become a validator and abuse this privilege.

Vulnerability Detail

Consider the following attack vectors:

1) Profiting unfairly from liquidation. Assume the current mark price for WETH is 2000. A user is liquidated and 100 ETH needs to be sold. Assume there are available orders that can match this liquidation at 2000. Instead of matching it directly, the validator can create his own order at 1980 and match the liquidation to his own order. He can then immediately sell and match with the other available orders for a profit of 2,000 (20 * 100) USDC.

2) Manipulation of funding rate. Validators are allowed to match non-liquidation orders at up to 20% spread from the oracle. This makes it incredibly easy for validators to manipulate the markPriceTwap. By creating and matching tiny orders at the extremes of this spread they can dramatically skew the funding rate it whatever way they please. Max funding rates can liquidate leveraged positions very quickly allowing that validator to further profit from their liquidations.

Now we can discuss how just about anyone can become a validator and abuse this privilege with zero consequences.

First we need to consider the typical methodology for ensuring validators behave and why NONE of those factors apply in this scenario. 1) "Slash validators that misbehave." Hubble mainnet is a fork of the AVAX C-Chain which is different from most chains in the fact that AVAX validators can't be slashed. 2) "Validators are forced to hold a volatile protocol token that would depreciate if they are publicly observed misbehaving." On Hubble mainnet the gas token is USDC so it would not depreciate in the event that validators misbehave. 3) "Blocks that break consensus rules are rejected." The Hubble exchange smart contracts are not part of the consensus layer so abusing validator privilege as described aren't breaking any consensus rules and would therefore be accepted by honest validators.

Second we consider the ease of becoming a validator. Hubble mainnet is a fork of AVAX and uses it's same consensus mechanism. This allows any node who posts the required stake to become a validator for the network. This allows anyone with access to any decent level of capital to become a validator and begin exploiting users.

Impact

Validators can manipulate funding rates and profit unfairly from liquidations

Code Snippet

OrderBook.sol#L215-L258

Tool used

Manual Review

Recommendation

The methodology of order matching needs to be rethought to force validators to match fairly and efficiently

0xshinobii commented 1 year ago

We will fix this with post-mainnet releases. Initially, we are launching with a trusted, closed set of validators and will fix this before we open for public validators. Remarks about point 2. Manipulation of funding rate - for this to happen, a validator will need to place and execute orders for a fairly large amount of time if there are other trades in the system. So this scenario can happen in case of low liquidity in the system.

ghost commented 1 year ago

Escalate

This should not be a high because validators in hubble are decided by the governance which is different from the validators who secure the hubble network.

For this exploit to occur, an existing validator needs to be compromised or a rogue validator added by manipulating the governance vote. Moreover, this attack assumes that there is only 1 validator when there can be multiple non rogue validators matching orders as well.

sherlock-admin2 commented 1 year ago

Escalate

This should not be a high because validators in hubble are decided by the governance which is different from the validators who secure the hubble network.

For this exploit to occur, an existing validator needs to be compromised or a rogue validator added by manipulating the governance vote. Moreover, this attack assumes that there is only 1 validator when there can be multiple non rogue validators matching orders as well.

You've created a valid escalation!

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.

djb15 commented 1 year ago

I agree with this escalation. Regarding the manipulation of the funding rate see https://github.com/sherlock-audit/2023-04-hubble-exchange-judging/issues/100#issuecomment-1640389572 which is acknowledged as a low severity issue because you just need 1 non-malicious validator. I believe the same is true for this report where one non malicious validator would prevent the funding rate being manipulated over a significant period of time (in normal market conditions).

EDIT: See https://github.com/sherlock-audit/2023-04-hubble-exchange-judging/issues/87#issuecomment-1644045278 for some related justification

ctf-sec commented 1 year ago

In the part funding manipulation yes

but the attacker vector does not require all validator to be malicious, one malicious validator can do this and extract value from user.

Profiting unfairly from liquidation. Assume the current mark price for WETH is 2000. A user is liquidated and 100 ETH needs to be sold. Assume there are available orders that can match this liquidation at 2000. Instead of matching it directly, the validator can create his own order at 1980 and match the liquidation to his own order. He can then immediately sell and match with the other available orders for a profit of 2,000 (20 * 100) USDC.

Recommend maintaining high severity

djb15 commented 1 year ago

That's a fair point, I've found the following quote from the sponsor in the discord channel (https://discord.com/channels/812037309376495636/1121092175216787507/1126818439646957628):

So, in that sense and from the perspective of the smart contract audit, validators are not trusted.

So I agree the "profit unfairly from liquidations" part of this report is probably a valid high as I agree you only need 1 malicious validator. But the "manipulating funding rates part" is probably a medium as you would need either lots of malicious validators in normal market conditions or 1 malicious validator in low liquidity market conditions. So high overall :D

IAm0x52 commented 1 year ago

The validators aren't different, they are the same. The concept that governance would restrict validators to a trusted set is new information that was never present in any of the docs or readme during the contest. All information indicated that validators would be open and decentralized. Based on that information anyone could be a validator which is why I think this should remain as a high risk issue.

87 should not be a dupe of this issue. Realistically I shouldn't have submitted this as a single issue and should have submitted it as two. One for liquidation manipulation and the other for the funding rate manipulation. Since a single issue can't be a dupe of two different issues, this should remain a separate issue.

ghost commented 1 year ago

The validators aren't different, they are the same.

If they are the same and any validators of the network can participate in matching orders, then why was this information not communicated to us on the docs or the README?

Can you please show me an example of a smart contract on AVAX C-Chain with a protected "validator" role in the smart contract and yet any of the validators of AVAX consensus layer can pass that check.

More generally, should vulnerabilities regarding information that was not communicated to us be also considered valid? This encompasses literally everything, and you start to go into the world of crazy hypothetical what-ifs. @hrishibhat

All information indicated that validators would be open and decentralized.

The README clearly states that there is a governance role which is a TRUSTED role. All actions of a trusted role should be trusted hence one would assume that who the governance selects as validators are to be trusted.

If this premise is wrong, then one can go so far as to generalise that all privileged roles cannot be trusted because there is a non-zero chance that they will go rogue / fall in the hands of the wrong people.

Realistically I shouldn't have submitted this as a single issue and should have submitted it as two. One for liquidation manipulation and the other for the funding rate manipulation.

Both liquidation manipulation and funding rate manipulation is predicated on the assumption that rogue validators were selected to match orders by the governance.

Liquidation manipulation also assumes that:

  1. there are no other honest validators to match this user.
  2. the rogue validator of the smart contract also controls a substantial AVAX stake as a node at the consensus layer in order to ensure that his orders are finalized.

As described by the sponsor, funding rate manipulation can only occur when there is low liquidity. When there is high liquidity, manipulating the TWAP price will be too expensive just to extract funding from users. In many past contests, a comment like this by the sponsor would immediately invalidate the report.

It is also by design that if the system has low liquidity, the funding paid by the user will be high. The onus should be on the user to close his position.

Since a single issue can't be a dupe of two different issues, this should remain a separate issue.

Lol I'm gonna start grouping all my issues together too because multiple issues cant be a dup of a single issue. 🤷🏼‍♂️

IAm0x52 commented 1 year ago

If they are the same and any validators of the network can participate in matching orders, then why was this information not communicated to us on the docs or the README

All network validators have to be added by governance or else there would be a lot of empty blocks because the unverified block producers wouldn't be able to match any orders. Fundamentally they have to be the same.

there are no other honest validators to match this user.

This statement is incorrect. When it is this validator's turn to produce the block they will be able to abuse this.

the rogue validator of the smart contract also controls a substantial AVAX stake as a node at the consensus layer in order to ensure that his orders are finalized.

Also incorrect. If you would read my issue, abusing liquidations in this way doesn't break any consensus rules and therefore would be accepted by honest validators.

Both liquidation manipulation and funding rate manipulation is predicated on the assumption that rogue validators were selected to match orders by the governance.

Incorrect again. Anyone, not just validators, can manipulate funding rate as demonstrated by #87. Hence why this is two separate issues.

Lol I'm gonna start grouping all my issues together too because multiple issues cant be a dup of a single issue. 🤷🏼‍♂️

Yet another incorrect statement. In this scenario I receive less of the prize pool not more.

hrishibhat commented 1 year ago

@P12473 I think the above response makes sense. do you have further comments?

ghost commented 1 year ago

@P12473 I think the above response makes sense. do you have further comments?

Thanks for giving me this opportunity to speak but I do not have any comments.

@IAm0x52 is a much better auditor than I am and it is most likely that I am wrong. I want to understand his perspective but I am unable to do so from his explanation. If you agree with his take then I will rest my case.

I only hope that the decision was an impartial one. Thanks again!

0xshinobii commented 1 year ago

For the scope of this audit, validators should not be trusted. However, we'll launch on mainnet with a closed set of trusted validators and plan to open for public validators later. So, point 1 (liquidation) is a valid high issue as it can happen even if 1 validator is malicious and point 2 (funding payment) is a valid medium issue as it can happen only in low liquidity case and will also be costly to do so.

hrishibhat commented 1 year ago

Result: High Unique Considering this issue a valid high based on the discussion above and Sponsor's comment

Additionally, the readme shared information about the trusted protocol roles were limited to the governance. And the docs provided context on the validators and their functioning: https://medium.com/hubbleexchange/introducing-hubble-exchange-a-purpose-built-chain-for-perps-25c60db66d44 https://hubbleexchange.notion.site/Hubble-Exchange-Overview-e9487ec19fe9451c9d445be15978c740

sherlock-admin2 commented 1 year ago

Escalations have been resolved successfully!

Escalation status:

0xshinobii commented 1 year ago

As mentioned in the above comment, we will fix this in later releases