Canto Dex Oracle contest - CertoraInc's results

Lines of code

https://github.com/code-423n4/2022-09-canto/blob/65fbb8b9de22cf8f8f3d742b38b4be41ee35c468/src/Swap/BaseV1-periphery.sol#L582

Vulnerability details

Impact

Note: I'll use the notation decimals here to represent 10 ** token.decimals(), just like th variable defined in the BaseV1Router.getPriceLP() function.

In the BaseV1Router.getPriceLP() function, the token0 TVL is calculated in terms of NOTE, by multiplying the TVL in terms of TOKEN, by the price of it in terms of NOTE and dividing by decimals.

uint token0TVL = assetReserves[i] * (prices[i] / decimals);

However, the price is first divided by decimals first, and then multiplied by the TVL, which can cause data loss in the division. We have 2 cases of data loss here:

1. If we are "lucky" and the price is greater than decimals, then some data can get loss, but the TVL will remain positive.
2. In this case we are unlucky and the price is less than decimals. Here the calculation of prices[i] / decimals will be zero and the whole TVL calculation will be zero.

Now, this calculation is done 8 times, for each sample of the TVLs and prices, so each data loss here has 1/8 of the actual effect to the TVL. However, most of the time the price won't change that match (because the samples are taken every 30 minutes) and the average will be zero very fast.

The low TVL can lower the LP token's price, and can cause collateral value loss and probably wrong liquidations.

Proof of Concept

Let's assume the TVL (in one sample) in the CANTO/NOTE pool is 50e18. Now I'll show an example for both of the cases:

1. The (sampled) price is 2.5 NOTE per CANTO (> 1e18) (i.e. the price will be 2.5e18). The TVL in terms of NOTE will be calculated as such: 50e18 * (2.5e18 / 1e18) = 50e18 * 2 = 100e18 The TVL in this sample will be 100e18 instead of 125e18, which is pretty bad.
2. The (sampled) price is 0.5 NOTE per CANTO (<> 1e18) (i.e. the price will be 0.5e18 = 5e17). The TVL in terms of NOTE will be calculated as such: 50e18 * (0.5e18 / 1e18) = 50e18 * 0 = 0 The TVL in this sample will be 0 instead of 50e18, which is also pretty bad.

Tools Used

Manual audit

Recommended Mitigation Steps

Multiply before you divide, that way no data will be lost:

uint token0TVL = (assetReserves[i] * prices[i]) / decimals;

Low and Non Critical Issues

• Pragmas should be locked to a specific compiler version, to avoid contracts getting deployed using a different version, which may have a greater risk of undiscovered bugs.
• Internal functions without _ mislead the reader. It is better to mark internal functions by adding _ in the beginning of their name.