Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Raunak/gas griefing fixes #91

Merged
merged 4 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions contracts/core/Dispatcher.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand Down
1 change: 1 addition & 0 deletions contracts/libs/Ibc.sol
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ library IBCErrors {
error consensusStateVerificationFailed();
error packetNotTimedOut();
error invalidAddress();
error notEnoughGas();

// packet sequence related errors.
error invalidPacketSequence();
Expand Down
40 changes: 40 additions & 0 deletions test/Dispatcher.gasGriefing.t.sol
Original file line number Diff line number Diff line change
@@ -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
}
}
52 changes: 52 additions & 0 deletions test/mocks/GasUsingMars.sol
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
Loading