From 43f1466f26d3ddced53c92f3157f557193184eac Mon Sep 17 00:00:00 2001 From: pascal Date: Mon, 17 Jun 2024 15:48:08 +0200 Subject: [PATCH] tests: Adds integration test to prove non OOG manipulative --- test/Aggor.t.sol | 30 ++-- .../AggorIntegration_eth_ETH_USD.t.sol | 151 ++++++++++++++++++ 2 files changed, 166 insertions(+), 15 deletions(-) diff --git a/test/Aggor.t.sol b/test/Aggor.t.sol index 810f427..6a1869f 100644 --- a/test/Aggor.t.sol +++ b/test/Aggor.t.sol @@ -796,15 +796,15 @@ contract AggorTest is Test { /// @dev This test verifies that any gas less than necessary given to a /// read call will lead to a revert. function test_read_RevertsIf_OutOfGas_ExplicitBoundaryChecks() public { - uint valChr = 1 * 1e18; - uint valChl = 1 * 1e8; + uint valChr = 1e18; + uint valChl = 1e8; ChronicleMock(chronicle).setValAndAge(valChr, block.timestamp); ChainlinkMock(chainlink).setValAndAge( int(uint(valChl)), block.timestamp ); - uint wantVal = 1 * 1e8; + uint wantVal = 1e8; uint wantAge = block.timestamp; IAggor.Status memory wantStatus = IAggor.Status({path: 2, goodOracleCtr: 2}); @@ -863,22 +863,22 @@ contract AggorTest is Test { aggor.readWithStatus{gas: gasUsage_readWithStatus - 1}(); } - /// @dev This test verifies that, given enough gas, a Chronicle OOO does + /// @dev This test verifies that, given enough gas, a Chronicle OOG does /// not lead to a total revert. - function test_read_DoesNotAlwaysRevertIf_ChronicleOOO() public { - uint valChr = 2 * 1e18; - uint valChl = 1 * 1e8; + function test_read_DoesNotAlwaysRevertIf_ChronicleOOG() public { + uint valChr = 2e18; + uint valChl = 1e8; ChronicleMock(chronicle).setValAndAge(valChr, block.timestamp); ChainlinkMock(chainlink).setValAndAge( int(uint(valChl)), block.timestamp ); - // Let Chronicle run into OOO. + // Let Chronicle run into OOG. ChronicleMock(chronicle).setBurnGas(true); // Expect only Chainlink's value. - uint wantVal = 1 * 1e8; + uint wantVal = 1e8; uint wantAge = block.timestamp; IAggor.Status memory wantStatus = IAggor.Status({path: 4, goodOracleCtr: 1}); @@ -886,22 +886,22 @@ contract AggorTest is Test { _checkReadFunctions(wantVal, wantAge, wantStatus); } - /// @dev This test verifies that, given enough gas, a Chainlink OOO does + /// @dev This test verifies that, given enough gas, a Chainlink OOG does /// not lead to a total revert. - function test_read_DoesNotAlwaysRevertIf_ChainlinkOOO() public { - uint valChr = 2 * 1e18; - uint valChl = 1 * 1e8; + function test_read_DoesNotAlwaysRevertIf_ChainlinkOOG() public { + uint valChr = 2e18; + uint valChl = 1e8; ChronicleMock(chronicle).setValAndAge(valChr, block.timestamp); ChainlinkMock(chainlink).setValAndAge( int(uint(valChl)), block.timestamp ); - // Let Chainlink run into OOO. + // Let Chainlink run into OOG. ChainlinkMock(chainlink).setBurnGas(true); // Expect only Chronicle's value (in 8 decimals). - uint wantVal = 2 * 1e8; + uint wantVal = 2e8; uint wantAge = block.timestamp; IAggor.Status memory wantStatus = IAggor.Status({path: 4, goodOracleCtr: 1}); diff --git a/test/integration/AggorIntegration_eth_ETH_USD.t.sol b/test/integration/AggorIntegration_eth_ETH_USD.t.sol index 136ac63..2f2d19a 100644 --- a/test/integration/AggorIntegration_eth_ETH_USD.t.sol +++ b/test/integration/AggorIntegration_eth_ETH_USD.t.sol @@ -220,6 +220,157 @@ contract AggorIntegrationTest_eth_ETH_USD is Test { assertEq(gotStatus.path, wantStatus.path); assertEq(gotStatus.goodOracleCtr, wantStatus.goodOracleCtr); } + + /// @dev Verifies that its not possible to manipulate Aggor's value + /// derivation path in `latestAnswer()` to prevent the Chainlink + /// oracle from being read via capping the forwarded gas. + /// + /// @dev Note to execute against local forked anvil node in order to + /// prevent rate-limits. + function test_latestAnswer_OOGAttack_PreventChainlink() public { + uint128 chrVal = 2e18; + uint128 chlVal = 1e8; + + // Set oracles. + _setChronicle(chrVal, uint32(block.timestamp)); + _setChainlink(chlVal, uint32(block.timestamp)); + + // Expect only Chronicle's value (in 8 decimals). + uint wantVal = 2e8; + + // Call read function once to warm slots. + aggor.latestAnswer(); + + // Gas usage is an upper bound based on following solc options: + // version : 0.8.16 + // optimizer : true + // optimizer_runs : 10_000 + // via-ir : false + uint gasUsage = 9000; + while (gasUsage > 500) { + try aggor.latestAnswer{gas: gasUsage}() returns (int answer) { + // Call returned, ie no OOG. + // Verify whether Chainlink call was executed. + bool chainlinkExecuted = uint(answer) != wantVal; + if (!chainlinkExecuted) { + // Success! Found a gas cap to execute Chronicle call but + // not Chainlink, while at the same providing enough gas + // for Aggor to return. + revert("latestAnswer() vulnerable to OOG attack"); + } else { + // Gas cap was too much. Function executed normally. + gasUsage -= 1; + } + } catch { + // Only possible due to OOG. + // If this happens we provided too little gas for the call to + // succeed. + break; + } + } + } + + /// @dev Verifies that its not possible to manipulate Aggor's value + /// derivation path in `latestRoundData()` to prevent the Chainlink + /// oracle from being read via capping the forwarded gas. + /// + /// @dev Note to execute against local forked anvil node in order to + /// prevent rate-limits. + function test_latestRoundData_OOGAttack_PreventChainlink() public { + uint128 chrVal = 2e18; + uint128 chlVal = 1e8; + + // Set oracles. + _setChronicle(chrVal, uint32(block.timestamp)); + _setChainlink(chlVal, uint32(block.timestamp)); + + // Expect only Chronicle's value (in 8 decimals). + uint wantVal = 2e8; + + // Call read function once to warm slots. + aggor.latestRoundData(); + + // Gas usage is an upper bound based on following solc options: + // version : 0.8.16 + // optimizer : true + // optimizer_runs : 10_000 + // via-ir : false + uint gasUsage = 10_000; + while (gasUsage > 500) { + try aggor.latestRoundData{gas: gasUsage}() returns ( + uint80, int answer, uint, uint, uint80 + ) { + // Call returned, ie no OOG. + // Verify whether Chainlink call was executed. + bool chainlinkExecuted = uint(answer) != wantVal; + if (!chainlinkExecuted) { + // Success! Found a gas cap to execute Chronicle call but + // not Chainlink, while at the same providing enough gas + // for Aggor to return. + revert("latestRoundData() vulnerable to OOG attack"); + } else { + // Gas cap was too much. Function executed normally. + gasUsage -= 1; + } + } catch { + // Only possible due to OOG. + // If this happens we provided too little gas for the call to + // succeed. + break; + } + } + } + + /// @dev Verifies that its not possible to manipulate Aggor's value + /// derivation path in `readWithStatus()` to prevent the Chainlink + /// oracle from being read via capping the forwarded gas. + /// + /// @dev Note to execute against local forked anvil node in order to + /// prevent rate-limits. + function test_readWithStatus_OOGAttack_PreventChainlink() public { + uint128 chrVal = 2e18; + uint128 chlVal = 1e8; + + // Set oracles. + _setChronicle(chrVal, uint32(block.timestamp)); + _setChainlink(chlVal, uint32(block.timestamp)); + + // Expect only Chronicle's value (in 8 decimals). + uint wantVal = 2e8; + + // Call read function once to warm slots. + aggor.readWithStatus(); + + // Gas usage is an upper bound based on following solc options: + // version : 0.8.16 + // optimizer : true + // optimizer_runs : 10_000 + // via-ir : false + uint gasUsage = 10_000; + while (gasUsage > 500) { + try aggor.readWithStatus{gas: gasUsage}() returns ( + uint val, uint, IAggor.Status memory + ) { + // Call returned, ie no OOG. + // Verify whether Chainlink call was executed. + bool chainlinkExecuted = val != wantVal; + if (!chainlinkExecuted) { + // Success! Found a gas cap to execute Chronicle call but + // not Chainlink, while at the same providing enough gas + // for Aggor to return. + revert("readWithStatus() vulnerable to OOG attack"); + } else { + // Gas cap was too much. Function executed normally. + gasUsage -= 1; + } + } catch { + // Only possible due to OOG. + // If this happens we provided too little gas for the call to + // succeed. + break; + } + } + } } interface IChainlinkAggregatorV3_Aggregator {