From 5593b600a3d123446aab518e2b5a7cf89a1d4347 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Tue, 23 Jul 2024 10:44:12 +0900 Subject: [PATCH] fix port identifier validation Signed-off-by: Jun Kimura --- .gas-snapshot | 21 ++++++++++---------- contracts/core/24-host/IBCHostLib.sol | 28 +++++++++++++++------------ 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index f7d52883..0c68f54a 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,5 +1,5 @@ IBCMockAppTest:testHandshake() (gas: 4420576) -IBCMockAppTest:testHandshakeBetweenDifferentPorts() (gas: 3339589) +IBCMockAppTest:testHandshakeBetweenDifferentPorts() (gas: 3339603) IBCMockAppTest:testPacketRelay() (gas: 13935831) IBCMockAppTest:testPacketTimeout() (gas: 4284259) IBCTest:testBenchmarkCreateMockClient() (gas: 233366) @@ -8,13 +8,14 @@ IBCTest:testBenchmarkRecvPacket() (gas: 158888) IBCTest:testBenchmarkSendPacket() (gas: 128432) IBCTest:testBenchmarkUpdateMockClient() (gas: 160229) IBCTest:testToUint128((uint64,uint64)) (runs: 256, μ: 947, ~: 947) -TestICS02:testCreateClient() (gas: 36633817) -TestICS02:testInvalidCreateClient() (gas: 36530934) -TestICS02:testInvalidUpdateClient() (gas: 36529944) -TestICS02:testRegisterClient() (gas: 36185610) -TestICS02:testRegisterClientDuplicatedClientType() (gas: 36170919) -TestICS02:testRegisterClientInvalidClientType() (gas: 36200380) -TestICS02:testUpdateClient() (gas: 36698144) +ICS24HostTest:testValidatePortIdentifier() (gas: 37060) +TestICS02:testCreateClient() (gas: 36639835) +TestICS02:testInvalidCreateClient() (gas: 36536946) +TestICS02:testInvalidUpdateClient() (gas: 36535962) +TestICS02:testRegisterClient() (gas: 36191628) +TestICS02:testRegisterClientDuplicatedClientType() (gas: 36176937) +TestICS02:testRegisterClientInvalidClientType() (gas: 36206398) +TestICS02:testUpdateClient() (gas: 36704162) TestICS03Handshake:testConnOpenAck() (gas: 1858230) TestICS03Handshake:testConnOpenConfirm() (gas: 2054143) TestICS03Handshake:testConnOpenInit() (gas: 1429838) @@ -29,7 +30,7 @@ TestICS03Version:testIsSupportedVersion() (gas: 7864) TestICS03Version:testPickVersion() (gas: 25327) TestICS03Version:testVerifyProposedVersion() (gas: 11777) TestICS03Version:testVerifySupportedFeature() (gas: 4153) -TestICS04Handshake:testBindPort() (gas: 458549) +TestICS04Handshake:testBindPort() (gas: 458623) TestICS04Handshake:testChanClose() (gas: 12973942) TestICS04Handshake:testChanOpenAck() (gas: 3459492) TestICS04Handshake:testChanOpenConfirm() (gas: 3770761) @@ -54,7 +55,7 @@ TestICS04Upgrade:testUpgradeCrossingHelloIncompatibleProposals() (gas: 5068281) TestICS04Upgrade:testUpgradeFull() (gas: 56559261) TestICS04Upgrade:testUpgradeInit() (gas: 3074224) TestICS04Upgrade:testUpgradeNoChanges() (gas: 2473090) -TestICS04Upgrade:testUpgradeNotUpgradableModule() (gas: 3648610) +TestICS04Upgrade:testUpgradeNotUpgradableModule() (gas: 3648638) TestICS04Upgrade:testUpgradeOutOfSync() (gas: 3905908) TestICS04Upgrade:testUpgradeRelaySuccessAtCounterpartyFlushComplete() (gas: 5230956) TestICS04Upgrade:testUpgradeRelaySuccessAtFlushing() (gas: 5604165) diff --git a/contracts/core/24-host/IBCHostLib.sol b/contracts/core/24-host/IBCHostLib.sol index 4df51a54..42dfe66d 100644 --- a/contracts/core/24-host/IBCHostLib.sol +++ b/contracts/core/24-host/IBCHostLib.sol @@ -4,31 +4,35 @@ pragma solidity ^0.8.20; library IBCHostLib { /** * @dev validatePortIdentifier validates a port identifier string - * check if the string consist of characters in one of the following categories only: - * - Alphanumeric - * - `.`, `_`, `+`, `-`, `#` - * - `[`, `]`, `<`, `>` + * check if the string consist of characters in one of the following categories only: + * - Alphanumeric + * - `.`, `_`, `+`, `-`, `#` + * - `[`, `]`, `<`, `>` + * + * The validation is based on the ibc-go implementation: + * https://github.com/cosmos/ibc-go/blob/b0ed0b412ea75e66091cc02746c95f9e6cf4445d/modules/core/24-host/validate.go#L86 */ function validatePortIdentifier(bytes memory portId) internal pure returns (bool) { - if (portId.length < 2 || portId.length > 128) { + uint256 portIdLength = portId.length; + if (portIdLength < 2 || portIdLength > 128) { return false; } unchecked { - for (uint256 i = 0; i < portId.length; i++) { + for (uint256 i = 0; i < portIdLength; i++) { uint256 c = uint256(uint8(portId[i])); if ( // a-z - (c >= 0x61 && c <= 0x7A) + !(c >= 0x61 && c <= 0x7A) // 0-9 - || (c >= 0x30 && c <= 0x39) + && !(c >= 0x30 && c <= 0x39) // A-Z - || (c >= 0x41 && c <= 0x5A) + && !(c >= 0x41 && c <= 0x5A) // ".", "_", "+", "-" - || (c == 0x2E || c == 0x5F || c == 0x2B || c == 0x2D) + && !(c == 0x2E || c == 0x5F || c == 0x2B || c == 0x2D) // "#", "[", "]", "<", ">" - || (c == 0x23 || c == 0x5B || c == 0x5D || c == 0x3C || c == 0x3E) + && !(c == 0x23 || c == 0x5B || c == 0x5D || c == 0x3C || c == 0x3E) ) { - continue; + return false; } } }