Canto Dex Oracle contest - Respx's results

Lines of code

https://github.com/code-423n4/2022-09-canto/blob/65fbb8b9de22cf8f8f3d742b38b4be41ee35c468/src/Swap/BaseV1-core.sol#L224-L258 https://github.com/code-423n4/2022-09-canto/blob/65fbb8b9de22cf8f8f3d742b38b4be41ee35c468/src/Swap/BaseV1-core.sol#L271-L289 https://github.com/code-423n4/2022-09-canto/blob/65fbb8b9de22cf8f8f3d742b38b4be41ee35c468/src/Swap/BaseV1-core.sol#L187-L194 https://github.com/code-423n4/2022-09-canto/blob/65fbb8b9de22cf8f8f3d742b38b4be41ee35c468/src/Swap/BaseV1-core.sol#L201-L222 https://github.com/code-423n4/2022-09-canto/blob/65fbb8b9de22cf8f8f3d742b38b4be41ee35c468/src/Swap/BaseV1-periphery.sol#L487-L593

Vulnerability details

Description

There is insufficient resilience in the design for the case where there has been no call to _update() for a long time. Consider these possible scenarios:

1. A denial of service attack on the blockchain has prevented transactions occurring for a significant period of time.
2. An extreme spike in gas prices has prevented transactions occurring for a significant period of time.
3. Some unforeseen technical error takes the blockchain down, perhaps during an upgrade or fork. In any of these scenarios, it is possible that the price of any non-stablecoin token might be prone to serious volatility, $CANTO in particular.

The system has no provision for this issue.

Impact

Consider the following attack scenario:

1. Assume a period of network downtime, perhaps a DOS attack.
2. Assume a large drop in the price of $CANTO during this time.
3. Assume attacker is able to queue a transaction to be executed as soon as network service resumes (perhaps through producing a block themselves or high gas pricing). This transaction uses a lending system that relies on this oracle, and that lending system calls reserves() to calculate the TWAP price of $CANTO. The values returned are out of date and far too high. The attacker is then able to borrow stablecoins against their $CANTO at too high a rate.

Proof of Concept

https://github.com/code-423n4/2022-09-canto/blob/65fbb8b9de22cf8f8f3d742b38b4be41ee35c468/src/Swap/BaseV1-core.sol#L224-L258 https://github.com/code-423n4/2022-09-canto/blob/65fbb8b9de22cf8f8f3d742b38b4be41ee35c468/src/Swap/BaseV1-core.sol#L271-L289 reserves(), sampleReserves() and sampleSupply() make no use of block.timestamp. They all measure only the time duration of the observations. There is no awareness of how much time has passed since the most recent observation was made. https://github.com/code-423n4/2022-09-canto/blob/65fbb8b9de22cf8f8f3d742b38b4be41ee35c468/src/Swap/BaseV1-core.sol#L187-L194 https://github.com/code-423n4/2022-09-canto/blob/65fbb8b9de22cf8f8f3d742b38b4be41ee35c468/src/Swap/BaseV1-core.sol#L201-L222 https://github.com/code-423n4/2022-09-canto/blob/65fbb8b9de22cf8f8f3d742b38b4be41ee35c468/src/Swap/BaseV1-periphery.sol#L487-L593 quote() and sample() have the same logic, and these functions are called in getPriceCanto() and getPriceNote(). getPriceLP() relies on sampleReserves(). They therefore have the same issue.

Tools Used

Manual Inspection

Recommended Mitigation Steps

The system could track the average duration time of observations and, if any of the observations in a sample are significantly greater than this average, the system could either refuse to return a sample, or could return a warning flag to indicate that the sample data could be unreliable. There is precedent in the system for refusing to return a sample (see line 242 of BaseV1-core.sol).