diff --git a/contracts/core/Dispatcher.sol b/contracts/core/Dispatcher.sol index 26a489f9..618aa044 100644 --- a/contracts/core/Dispatcher.sol +++ b/contracts/core/Dispatcher.sol @@ -555,7 +555,18 @@ contract Dispatcher is OwnableUpgradeable, UUPSUpgradeable, IDispatcher { // Only call if we are sure receiver is a contract // Note: This tx won't revert if the low-level call fails, see // https://docs.soliditylang.org/en/latest/cheatsheet.html#members-of-address + uint256 gasBeforeCall = gasleft(); (success, message) = receiver.call(args); + + // x is the amount of gas left right before the low level call. Solidity will allocate x*63/64 to the low level + // call + // and x/64 for the remaining execution after the low-level call. If this low-level call runs out of gas, then + // gasLeft will be equal to x/64 at the start of the remaining execution. so we should check gasBefore < 1/64 + if (!success && gasleft() <= gasBeforeCall / 64) { + // Only check for out of gas if the call failed; if it was a successful call then it was gauranteed to not + // run out of gas + revert IBCErrors.notEnoughGas(); + } } function _authorizeUpgrade(address newImplementation) internal override onlyOwner {} diff --git a/contracts/libs/Ibc.sol b/contracts/libs/Ibc.sol index 5fa8537b..b78d8150 100644 --- a/contracts/libs/Ibc.sol +++ b/contracts/libs/Ibc.sol @@ -141,6 +141,7 @@ library IBCErrors { error consensusStateVerificationFailed(); error packetNotTimedOut(); error invalidAddress(); + error notEnoughGas(); // packet sequence related errors. error invalidPacketSequence(); diff --git a/test/Dispatcher.gasGriefing.t.sol b/test/Dispatcher.gasGriefing.t.sol new file mode 100644 index 00000000..83f31ac5 --- /dev/null +++ b/test/Dispatcher.gasGriefing.t.sol @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +import {Base} from "./Dispatcher.base.t.sol"; +import {GasUsingMars} from "./mocks/GasUsingMars.sol"; +import {IbcEndpoint, ChannelEnd, IbcUtils, IbcPacket, IBCErrors} from "../contracts/libs/Ibc.sol"; +import {TestUtilsTest} from "./TestUtils.t.sol"; + +contract DispatcherGasGriefing is Base { + IbcEndpoint src = IbcEndpoint("polyibc.bsc.58b604DB8886656695442374D8E940D814F2eDd4", "channel-99"); + IbcEndpoint dest; + bytes payload = bytes("msgPayload"); + bytes appAck = abi.encodePacked('{ "account": "account", "reply": "got the message" }'); + + GasUsingMars gasUsingMars; + ChannelEnd ch0 = + ChannelEnd("polyibc.eth.71C95911E9a5D330f4D621842EC243EE1343292e", IbcUtils.toBytes32("channel-0"), "1.0"); + ChannelEnd ch1 = + ChannelEnd("polyibc.eth.71C95911E9a5D330f4D621842EC243EE1343292e", IbcUtils.toBytes32("channel-1"), "1.0"); + + function setUp() public override { + (dispatcherProxy, dispatcherImplementation) = + TestUtilsTest.deployDispatcherProxyAndImpl(portPrefix, dummyConsStateManager); + gasUsingMars = new GasUsingMars(3_000_000, dispatcherProxy); // Set arbitrarily high gas useage in mars contract + } + + function test_GasGriefing() public { + IbcPacket memory packet; + packet.data = bytes("packet-1"); + packet.timeoutTimestamp = 15_566_401_733_896_437_760; + packet.dest.channelId = ch1.channelId; + packet.dest.portId = string(abi.encodePacked(portPrefix, IbcUtils.toHexStr(address(gasUsingMars)))); + packet.src.portId = ch0.portId; + packet.src.channelId = ch0.channelId; + packet.sequence = 1; + vm.expectRevert(abi.encodeWithSelector(IBCErrors.notEnoughGas.selector)); + dispatcherProxy.recvPacket{gas: 2_000_000}(packet, validProof); // Should be enough gas to run out in the + // callback but still finish execution + } +} diff --git a/test/mocks/GasUsingMars.sol b/test/mocks/GasUsingMars.sol new file mode 100644 index 00000000..a84aa7ca --- /dev/null +++ b/test/mocks/GasUsingMars.sol @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +import {IDispatcher} from "../../contracts/interfaces/IDispatcher.sol"; +import {Mars} from "../../contracts/examples/Mars.sol"; +import {IbcPacket, AckPacket} from "../../contracts/libs/Ibc.sol"; +import "forge-std/Test.sol"; + +/** + * Testing mock contrantract to test gas useage. + * This contract + * We can label the true gas cost of the parent transaction sent to the dispatcher as g = a + b + c . + * + * Solidity automatically allocates 63/64 of the remaining gas left in a tx for low level calls, so (g-a)*63/64 will be + * allocated for b, + * and the remaining (g-a)/64 will be allocated for c. + * If (g-a) * b > c and a malicious user sends a transaction with a gas limit g' such that a + c < g'< (g' - + * a)*(63/64), + * this can result in a gas griefing attack. + */ +contract GasUsingMars is Mars { + uint256 private gasToUse; + + constructor(uint256 _gasToUse, IDispatcher _ibcDispatcher) Mars(_ibcDispatcher) { + gasToUse = _gasToUse; + } + + function onRecvPacket(IbcPacket memory packet) + external + virtual + override + onlyIbcDispatcher + returns (AckPacket memory ackPacket) + { + recvedPackets.push(packet); + + _useGas(); + // solhint-disable-next-line quotes + return AckPacket(true, abi.encodePacked('{ "account": "account", "reply": "got the message" }')); + } + + function _useGas() internal view { + // This function is used to test the gas usage of the contract + uint256 startingGas = gasleft(); + uint256 dummyInt = 0; + + // It will use the gasToUse amount of gas + while (startingGas - gasleft() < gasToUse) { + dummyInt += 1; + } + } +}