VelodromeOracle is vulnerable to price manipulation attacks

[hats-bug-reporter[bot]]

Github username: @aviggiano Submission hash (on-chain): 0x0fbe6e02ec630eb007387f334a9b49ac5359a94b9427793340ff05612e621561 Severity: high severity

Description:

VelodromeOracle is vulnerable to price manipulation attacks

The function VelodromeOracle.get_lp_price makes an external call to IVeloPair.metadata and calculate the LP token price in terms of pool reserves. As a result, the oracle can be manipulated to change asset prices in VMEX and allow the attacker to borrow at inflated prices or to liquidate otherwise safe user positions.

Proof of Concept

In order to perform this exploit, the attacker can do the following:

1. Flash loan a pool token
2. Deposit into a Velodrome pair, hence manipulating the reserves
3. Liquidate users positions using the same Velodrome Pool as collateral

Severity

High. Attacker can borrow at inflated rates and liquidate otherwise safe positions due to unreliable price data.

GitHub issue

https://github.com/VMEX-finance/vmex/blob/3ad2b901118f49b25eb709fc8a098b96a3dce1ea/packages/contracts/contracts/protocol/oracles/VelodromeOracle.sol#L19

https://github.com/VMEX-finance/vmex/blob/a2eafb4d3294b92eb88ab79732778c2dc2e4e06d/packages/contracts/contracts/protocol/lendingpool/LendingPool.sol#L242

https://github.com/VMEX-finance/vmex/blob/b0dc00c5dd6bdcac05827128d14dcdc730f19e1c/packages/contracts/contracts/protocol/lendingpool/LendingPoolCollateralManager.sol#L337

Recommendation

Use an external price feed to fetch LP tokens instead of relying on the pool reserves.

[mel0ndev]

hi there, can you please include a POC where this is the case (preferably in solidity)? I am not sure what exactly you're referring to since the fair reserves model we're using accounts for reserves changing on chain.

[ksyao2002]

Also, please see this proof that our method of pricing is resistant to flash loan attacks: https://ethereum.stackexchange.com/questions/144866/manipulating-uniswap-v2s-pair-totalsupply-via-flash-loan

[aviggiano]

Hello,

@mel0ndev Thank you for reviewing my submission

I need more time to work on the Solidity POC (it is very time-consuming), but meanwhile please take a look at this article from cmichel (referenced in this talk from tincho from OpenZeppelin) that also utilizes the same formula shared by @ksyao2002.

In summary, the issue from VMEX implementation is that it does not follow an important recommendation from Stackoverflow/chichel, that of using TWAP prices instead of spot prices:

Notice how the price is now only a function of the TWAPs p0, p1 and k. Thus, using this calculation, swaps do not change the price of LP tokens and it seems like it would prevent these kinds of attacks that move along AMM curves.

p0 and p1 could be made flashloan-proof, as they could be pulled from Chainlink or Uniswap's TWAP price oracles.

Since VMEX is using the spot price for the calculation of LP price, and not the TWAP price, it should be possible to manipulate the LP price with flash loans.

[ksyao2002]

Thanks for your reply. The recommendation was to use TWAP if you use uniswap to query the spot price. We use chainlink oracles, which are flashloan resistant. See https://blog.alphaventuredao.io/fair-lp-token-pricing/. The requirement was to use an oracle that provides fair pricing, which could be chainlink spot price, or uniswap TWAP price. We only support tokens that have chainlink underlying.

Regardless, if you are able to find a POC that demonstrates this, feel free to comment it and we will reconsider your report.

[aviggiano]

Hi,

After reviewing the function code again, I agree that the prices used on the LP price calculation come from chain link, which makes my issue invalid.

Thanks for your response.