Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong trading pricing calculations #119

Open
code423n4 opened this issue Jun 30, 2021 · 1 comment
Open

Wrong trading pricing calculations #119

code423n4 opened this issue Jun 30, 2021 · 1 comment
Labels

Comments

@code423n4
Copy link
Contributor

Handle

0xsanson

Vulnerability details

Impact

In the Pricing contract, an agent can manipulate the trading prices by spamming an high amount of trades.

Indeed an agent can create an high amount of orders at an arbitrary price and with a near-zero amount (so the agent doesn't even need large funds); next he/she pairs the orders with another account and calls Trader.executeTrade; now every order calls a Pricing.recordTrade using the arbitrary price set by the agent.

Since the trades are all made in the same hour, by the way hourlyTracerPrices[currentHour] is calculated, it skews the average price towards the price set by the agent. This arbitrary value is used to calculate the fundingRates and the fairPrice, letting a malicious agent get the ability to manipulate the market.

Proof of Concept

https://github.com/code-423n4/2021-06-tracer/blob/main/src/contracts/Pricing.sol#L129

Tools Used

Manual analysis

Recommended Mitigation Steps

Pass the fillAmount parameter to recordTrade(...), and calculate hourlyTracerPrices[currentHour].trades summing fillAmount instead of 1 every trade.

@raymogg
Copy link
Collaborator

raymogg commented Jul 5, 2021

Issue is valid, and there appear to be a few other issues that reference similar problems.

The Trader contract will have a whitelist allowing only select relayers to push orders on chain. As long as off chain order books have sufficient liquidity, this issue is then mitigated as users can't just arbitrarily match orders and send them in, they must be matched on a book with liquidity. To alter the price you would then need to eat through significant liquidity (increasing the cost of this attack).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants