code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/oracles/BalancerV2LPOracle.sol#L88-L115

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/oracles/GUniLPOracle.sol#L91-L118

Vulnerability details

Oracle reports incorrect valuation of LP when the decimals from OracleA and OracleB decimals don't match

Impact

LP is overvalued allowing a malicious borrower to value more than the LP is worth

Proof of Concept

Assume a very simple pool with 10LP containing two assets, 1 of each valued at $1 each and that the oracle for asset A reports with 4 decimals and asset B with 3 decimals

That would give us the following inputs into _computerFairReserves() resA - 1e18 resB - 1e18 wA - 0.5 wB - 0.5 pxA - 10000 ($1 reported at 4 decimals) pxB - 1000 ($1 reported at 3 decimals)

Now we'll walk through _computerFairReserves() r0 = resA/resB = 1 r1 = (wApxB)/(wBpxA) = 0.1

r0 > r1 ratio = r1/r0 = 0.1

fairResA = = 0.316 fairResB = = 3.162

Plug that into the answer equation

answer = (fairResApxA + fairResBpxB) / total supply = 632 (3 decimal places)

The true value of the LP is calculated as: AmountAPriceA + AmountBPriceB = 200 (3 decimal places)

In this scenario it values the LP at ~3.16x it's true value

Tools Used

Excel

Recommended Mitigation Steps

Normalize the answer from the oracle to a standard number of decimals to avoid mismatch

kartoonjoy commented 2 years ago

Updated Lines of Code section per warden request in help desk ticket, https://www.notion.so/code4rena/Found-repeat-bug-99145d23b99e455a814366e03c6622b5.

m19 commented 2 years ago

We agree this issue exists but we don't think it's high risk because all Chainlink oracles are 8 decimals

gzeoneth commented 2 years ago

It would be nice to check the decimal but one could argue this is for gas optimization and gated by admin. Downgrading to Low / QA.