Closed pmerkleplant closed 3 months ago
I initially thought the TWAP call is also wrapped in a try-catch
which would need an analysis as well. However, looking more closely again the TWAP call is a normal external call and therefore not applicable to this analysis.
Intro
External calls can be wrapped into
try-catch
blocks leading to the following behaviour:catch
is executed if the call reverted or ran into OOG (out-of-gas)Note that OOG is (partially) controlled by the user, therefore a user may be able to manipulate the execution path towards the
catch
path. However, note that users can only control the gas of the full tx, ie not on a "per try-catch block" basis.SparkLend's
CappedFallbackRateSource
In SparkLend's CappedFallbackRateSource.sol this leads to an issue because the whole function's execution is just the
try-catch
block. Importantly, if thecatch
path is executed a default value is returned.Therefore, a user can provide just too little gas to prevent the external call to run into OOG, but still enough for the whole function to return - in this case the default value.
Note that a standard Solidity call forwards 63/64 of the available gas to the called contract. The 63/64 may not be enough for the external call- leading to OOG, while the 1/64 may be enough to terminate the function afterwards - leading to a non-failed execution.
Aggor
Both Oracles, Chronicle and Chainlink, are non-proxies (as in separating storage address and implementation) and for both the gas usage is capped.
Note, however, that Chainlink is using a "forwarding proxy". This means they could update their
aggregator
contract to an implementation that has uncapped gas usage. Nevertheless, this seems unlikely.This PR adds a couple of unit and integration tests to further define Aggor's behaviour wrt to capped gas usage.
The unit tests prove that any gas provided less than necessary leads to a normal OOG revert in Aggor.
The integration tests prove that it is not possible to provide such an amount of gas that the first oracle is read (Chronicle) but reading the second (Chainlink) fails due to OOG, while at the same time there's still enough gas left to finish Aggor's execution and returning just the Chronicle value. If this would be possible it would mean an attacker can manipulate Aggor's value derivation path via gas capping.