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

IBC-8: Fix missing checks on revision_number #310

Draft
wants to merge 5 commits into
base: audit-202409
Choose a base branch
from
Draft
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
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
FOUNDRY_PROFILE=default
FORGE=FOUNDRY_PROFILE=$(FOUNDRY_PROFILE) forge
SOLC_VERSION=0.8.24
EVM_VERSION=paris
EVM_VERSION=cancun
DOCKER=docker
ABIGEN="$(DOCKER) run -v .:/workspace -w /workspace -it ethereum/client-go:alltools-v1.11.6 abigen"
SOLHINT=npx solhint
Expand All @@ -16,7 +16,7 @@ TEST_UPGRADEABLE=false

.PHONY: build
build:
$(FORGE) build --sizes --skip test --use solc:$(SOLC_VERSION)
$(FORGE) build --sizes --skip test --use solc:$(SOLC_VERSION) --evm-version $(EVM_VERSION)

.PHONY: clean
clean:
Expand All @@ -33,7 +33,7 @@ lint:

.PHONY: test
test:
TEST_UPGRADEABLE=$(TEST_UPGRADEABLE) $(FORGE) test -vvvv --gas-report --isolate --use solc:$(SOLC_VERSION)
TEST_UPGRADEABLE=$(TEST_UPGRADEABLE) $(FORGE) test -vvvv --gas-report --isolate --use solc:$(SOLC_VERSION) --evm-version $(EVM_VERSION)

.PHONY: snapshot
snapshot:
Expand Down
3 changes: 2 additions & 1 deletion chains/ibft2/chain0/ibftConfigFile.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
"genesis": {
"config": {
"chainId": 2018,
"muirglacierblock": 0,
"cancunTime": 0,
"zeroBaseFee": true,
"ibft2": {
"blockperiodseconds": 1,
"epochlength": 30000,
Expand Down
3 changes: 2 additions & 1 deletion chains/ibft2/chain1/ibftConfigFile.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
"genesis": {
"config": {
"chainId": 3018,
"muirglacierblock": 0,
"cancunTime": 0,
"zeroBaseFee": true,
"ibft2": {
"blockperiodseconds": 1,
"epochlength": 30000,
Expand Down
2 changes: 1 addition & 1 deletion chains/qbft/chain0/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM hyperledger/besu:24.3.0
FROM hyperledger/besu:24.10.0

USER root

Expand Down
3 changes: 2 additions & 1 deletion chains/qbft/chain0/qbftConfigFile.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
"genesis": {
"config": {
"chainId": 2018,
"muirglacierblock": 0,
"cancunTime": 0,
"zeroBaseFee": true,
"qbft": {
"blockperiodseconds": 1,
"epochlength": 30000,
Expand Down
2 changes: 1 addition & 1 deletion chains/qbft/chain1/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM hyperledger/besu:24.3.0
FROM hyperledger/besu:24.10.0

USER root

Expand Down
3 changes: 2 additions & 1 deletion chains/qbft/chain1/qbftConfigFile.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
"genesis": {
"config": {
"chainId": 3018,
"muirglacierblock": 0,
"cancunTime": 0,
"zeroBaseFee": true,
"qbft": {
"blockperiodseconds": 1,
"epochlength": 30000,
Expand Down
31 changes: 25 additions & 6 deletions contracts/core/04-channel/IBCChannelPacketSendRecv.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ pragma solidity ^0.8.20;

import {Height} from "../../proto/Client.sol";
import {ConnectionEnd} from "../../proto/Connection.sol";
import {Channel} from "../../proto/Channel.sol";
import {Channel, Timeout} from "../../proto/Channel.sol";
import {ILightClient} from "../02-client/ILightClient.sol";
import {IIBCClientErrors} from "../02-client/IIBCClientErrors.sol";
import {IBCHeight} from "../02-client/IBCHeight.sol";
import {IBCChannelUpgradeBase} from "./IBCChannelUpgrade.sol";
import {IBCCommitment} from "../24-host/IBCCommitment.sol";
import {IBCModuleManager} from "../26-router/IBCModuleManager.sol";
import {IBCChannelLib} from "./IBCChannelLib.sol";
import {IIBCChannelPacketSendRecv} from "./IIBCChannel.sol";
import {IIBCChannelErrors} from "./IIBCChannelErrors.sol";
Expand All @@ -17,7 +17,7 @@ import {IIBCChannelErrors} from "./IIBCChannelErrors.sol";
* @dev IBCChannelPacketSendRecv is a contract that implements [ICS-4](https://github.com/cosmos/ibc/tree/main/spec/core/ics-004-channel-and-packet-semantics).
*/
contract IBCChannelPacketSendRecv is
IBCModuleManager,
IBCChannelUpgradeBase,
IIBCChannelPacketSendRecv,
IIBCChannelErrors,
IIBCClientErrors
Expand Down Expand Up @@ -132,9 +132,8 @@ contract IBCChannelPacketSendRecv is
revert IBCChannelUnexpectedPacketSource(msg_.packet.sourcePort, msg_.packet.sourceChannel);
}

if (msg_.packet.timeoutHeight.revision_height != 0 && block.number >= msg_.packet.timeoutHeight.revision_height)
{
revert IBCChannelTimeoutPacketHeight(block.number, msg_.packet.timeoutHeight.revision_height);
if (!msg_.packet.timeoutHeight.isZero() && hostHeight().gte(msg_.packet.timeoutHeight)) {
revert IBCChannelTimeoutPacketHeight(hostHeight(), msg_.packet.timeoutHeight);
}
if (msg_.packet.timeoutTimestamp != 0 && hostTimestamp() >= msg_.packet.timeoutTimestamp) {
revert IBCChannelTimeoutPacketTimestamp(hostTimestamp(), msg_.packet.timeoutTimestamp);
Expand Down Expand Up @@ -276,6 +275,26 @@ contract IBCChannelPacketSendRecv is
}

delete commitments[packetCommitmentKey];

if (channel.state == Channel.State.STATE_FLUSHING) {
Timeout.Data memory timeout = channelStorage.counterpartyUpgradeTimeout;
if (!timeout.height.isZero() || timeout.timestamp != 0) {
if (
!timeout.height.isZero() && hostHeight().gte(timeout.height)
|| timeout.timestamp != 0 && hostTimestamp() >= timeout.timestamp
) {
restoreChannel(msg_.packet.sourcePort, msg_.packet.sourceChannel, UpgradeHandshakeError.Timeout);
} else if (
canTransitionToFlushComplete(
channel.ordering, msg_.packet.sourcePort, msg_.packet.sourceChannel, channel.upgrade_sequence
)
) {
channel.state = Channel.State.STATE_FLUSHCOMPLETE;
updateChannelCommitment(msg_.packet.sourcePort, msg_.packet.sourceChannel, channel);
}
}
}

emit AcknowledgePacket(msg_.packet, msg_.acknowledgement);
lookupModuleByChannel(msg_.packet.sourcePort, msg_.packet.sourceChannel).onAcknowledgementPacket(
msg_.packet, msg_.acknowledgement, _msgSender()
Expand Down
48 changes: 34 additions & 14 deletions contracts/core/04-channel/IBCChannelPacketTimeout.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,21 @@ pragma solidity ^0.8.20;

import {Height} from "../../proto/Client.sol";
import {ConnectionEnd} from "../../proto/Connection.sol";
import {Channel, ChannelCounterparty} from "../../proto/Channel.sol";
import {Channel, ChannelCounterparty, Timeout} from "../../proto/Channel.sol";
import {ILightClient} from "../02-client/ILightClient.sol";
import {IBCHeight} from "../02-client/IBCHeight.sol";
import {IBCChannelLib} from "./IBCChannelLib.sol";
import {IBCChannelUpgradeBase} from "./IBCChannelUpgrade.sol";
import {IBCCommitment} from "../24-host/IBCCommitment.sol";
import {IBCModuleManager} from "../26-router/IBCModuleManager.sol";
import {IIBCChannelPacketTimeout} from "./IIBCChannel.sol";
import {IIBCChannelErrors} from "./IIBCChannelErrors.sol";

contract IBCChannelPacketTimeout is IBCModuleManager, IIBCChannelPacketTimeout, IIBCChannelErrors {
contract IBCChannelPacketTimeout is IBCChannelUpgradeBase, IIBCChannelPacketTimeout, IIBCChannelErrors {
using IBCHeight for Height.Data;

function timeoutPacket(MsgTimeoutPacket calldata msg_) external {
Channel.Data storage channel = getChannelStorage()[msg_.packet.sourcePort][msg_.packet.sourceChannel].channel;
ChannelStorage storage channelStorage = getChannelStorage()[msg_.packet.sourcePort][msg_.packet.sourceChannel];
Channel.Data storage channel = channelStorage.channel;
if (channel.state == Channel.State.STATE_UNINITIALIZED_UNSPECIFIED) {
revert IBCChannelUnexpectedChannelState(channel.state);
}
Expand Down Expand Up @@ -99,8 +100,6 @@ contract IBCChannelPacketTimeout is IBCModuleManager, IIBCChannelPacketTimeout,
msg_.proofHeight
);
}
channel.state = Channel.State.STATE_CLOSED;
updateChannelCommitment(msg_.packet.sourcePort, msg_.packet.sourceChannel);
} else if (channel.ordering == Channel.Order.ORDER_UNORDERED) {
bytes memory path = IBCCommitment.packetReceiptCommitmentPathCalldata(
msg_.packet.destinationPort, msg_.packet.destinationChannel, msg_.packet.sequence
Expand Down Expand Up @@ -129,6 +128,35 @@ contract IBCChannelPacketTimeout is IBCModuleManager, IIBCChannelPacketTimeout,
msg_.packet.sourcePort, msg_.packet.sourceChannel, msg_.packet.sequence
)];

if (channel.state == Channel.State.STATE_FLUSHING) {
Timeout.Data memory timeout = channelStorage.counterpartyUpgradeTimeout;
if (!timeout.height.isZero() || timeout.timestamp != 0) {
if (
!timeout.height.isZero() && hostHeight().gte(timeout.height)
|| timeout.timestamp != 0 && hostTimestamp() >= timeout.timestamp
) {
restoreChannel(msg_.packet.sourcePort, msg_.packet.sourceChannel, UpgradeHandshakeError.Timeout);
} else if (
canTransitionToFlushComplete(
channel.ordering, msg_.packet.sourcePort, msg_.packet.sourceChannel, channel.upgrade_sequence
)
) {
channel.state = Channel.State.STATE_FLUSHCOMPLETE;
updateChannelCommitment(msg_.packet.sourcePort, msg_.packet.sourceChannel, channel);
}
}
}

if (channel.ordering == Channel.Order.ORDER_ORDERED) {
if (channel.state == Channel.State.STATE_FLUSHING) {
delete channelStorage.upgrade;
deleteUpgradeCommitment(msg_.packet.sourcePort, msg_.packet.sourceChannel);
revertCounterpartyUpgrade(channelStorage);
}
channel.state = Channel.State.STATE_CLOSED;
updateChannelCommitment(msg_.packet.sourcePort, msg_.packet.sourceChannel, channel);
}

lookupModuleByChannel(msg_.packet.sourcePort, msg_.packet.sourceChannel).onTimeoutPacket(
msg_.packet, _msgSender()
);
Expand Down Expand Up @@ -290,12 +318,4 @@ contract IBCChannelPacketTimeout is IBCModuleManager, IIBCChannelPacketTimeout,
return (timeDelay + hostStorage.expectedTimePerBlock - 1) / hostStorage.expectedTimePerBlock;
}
}

/**
* @dev updateChannelCommitment updates the channel commitment for the given port and channel
*/
function updateChannelCommitment(string memory portId, string memory channelId) private {
getCommitments()[IBCCommitment.channelCommitmentKey(portId, channelId)] =
keccak256(Channel.encode(getChannelStorage()[portId][channelId].channel));
}
}
Loading