OOG attack analysis

[pmerkleplant]

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)
• the whole block reverts if the returndata decoding fails. This is only possible if proxies are called (not the case for Aggor)

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 the catch 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.

[hexonaut]

Looks good. Two minor issues.

[pmerkleplant]

Update re: TWAP OOG manipulation

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.