diff --git a/src/child/ChildERC20Bridge.sol b/src/child/ChildERC20Bridge.sol index 9a9e6212..d4eae07a 100644 --- a/src/child/ChildERC20Bridge.sol +++ b/src/child/ChildERC20Bridge.sol @@ -109,12 +109,13 @@ contract ChildERC20Bridge is if (!Strings.equal(messageSourceChain, rootChain)) { revert InvalidSourceChain(); } - if (!Strings.equal(sourceAddress, rootERC20BridgeAdaptor)) { revert InvalidSourceAddress(); } - if (data.length == 0) { - revert InvalidData(); + if (data.length <= 32) { + // Data must always be greater than 32. + // 32 bytes for the signature, and at least some information for the payload + revert InvalidData("Data too short"); } if (bytes32(data[:32]) == MAP_TOKEN_SIG) { @@ -122,7 +123,7 @@ contract ChildERC20Bridge is } else if (bytes32(data[:32]) == DEPOSIT_SIG) { _deposit(data[32:]); } else { - revert InvalidData(); + revert InvalidData("Unsupported action signature"); } } diff --git a/src/interfaces/child/IChildERC20Bridge.sol b/src/interfaces/child/IChildERC20Bridge.sol index 087daa89..aaf4195d 100644 --- a/src/interfaces/child/IChildERC20Bridge.sol +++ b/src/interfaces/child/IChildERC20Bridge.sol @@ -73,7 +73,7 @@ interface IChildERC20BridgeErrors { /// @notice Error when a message is given to the bridge from an address not the designated bridge adaptor. error NotBridgeAdaptor(); /// @notice Error when the message's payload is not valid. - error InvalidData(); + error InvalidData(string reason); /// @notice Error when the message's source chain is not valid. error InvalidSourceChain(); /// @notice Error when the source chain's message sender is not a recognised address. diff --git a/src/interfaces/root/IRootERC20Bridge.sol b/src/interfaces/root/IRootERC20Bridge.sol index 13bfb1e8..51d235d0 100644 --- a/src/interfaces/root/IRootERC20Bridge.sol +++ b/src/interfaces/root/IRootERC20Bridge.sol @@ -106,7 +106,7 @@ interface IRootERC20BridgeErrors { /// @notice Error when the given child chain bridge adaptor is invalid. error InvalidChildERC20BridgeAdaptor(); /// @notice Error when a message received has invalid data. - error InvalidData(); + error InvalidData(string reason); /// @notice Error when a message received has invalid source address. error InvalidSourceAddress(); /// @notice Error when a message received has invalid source chain. diff --git a/src/root/RootERC20Bridge.sol b/src/root/RootERC20Bridge.sol index bf72034d..266777a8 100644 --- a/src/root/RootERC20Bridge.sol +++ b/src/root/RootERC20Bridge.sol @@ -164,14 +164,16 @@ contract RootERC20Bridge is if (!Strings.equal(sourceAddress, childBridgeAdaptor)) { revert InvalidSourceAddress(); } - if (data.length == 0) { - revert InvalidData(); + if (data.length <= 32) { + // Data must always be greater than 32. + // 32 bytes for the signature, and at least some information for the payload + revert InvalidData("Data too short"); } if (bytes32(data[:32]) == WITHDRAW_SIG) { _withdraw(data[32:]); } else { - revert InvalidData(); + revert InvalidData("Unsupported action signature"); } } diff --git a/test/integration/child/ChildAxelarBridge.t.sol b/test/integration/child/ChildAxelarBridge.t.sol index c73f41a6..bf8e0da2 100644 --- a/test/integration/child/ChildAxelarBridge.t.sol +++ b/test/integration/child/ChildAxelarBridge.t.sol @@ -83,7 +83,7 @@ contract ChildERC20BridgeIntegrationTest is Test, IChildERC20BridgeEvents, IChil bytes32 commandId = bytes32("testCommandId"); bytes memory payload = abi.encode("invalid payload"); - vm.expectRevert(InvalidData.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidData.selector, "Unsupported action signature")); childAxelarBridgeAdaptor.execute(commandId, ROOT_CHAIN_NAME, ROOT_ADAPTOR_ADDRESS, payload); } @@ -109,7 +109,7 @@ contract ChildERC20BridgeIntegrationTest is Test, IChildERC20BridgeEvents, IChil bytes32 commandId = bytes32("testCommandId"); bytes memory payload = ""; - vm.expectRevert(InvalidData.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidData.selector, "Data too short")); childAxelarBridgeAdaptor.execute(commandId, ROOT_CHAIN_NAME, ROOT_ADAPTOR_ADDRESS, payload); } diff --git a/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol b/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol index 70b0d140..bedf6d97 100644 --- a/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol +++ b/test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol @@ -87,7 +87,7 @@ contract RootERC20BridgeWithdrawIntegrationTest is bytes32 commandId = bytes32("testCommandId"); string memory sourceAddress = rootBridge.childBridgeAdaptor(); - vm.expectRevert(InvalidData.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidData.selector, "Data too short")); axelarAdaptor.execute(commandId, CHILD_CHAIN_NAME, sourceAddress, data); } @@ -97,7 +97,7 @@ contract RootERC20BridgeWithdrawIntegrationTest is bytes32 commandId = bytes32("testCommandId"); string memory sourceAddress = rootBridge.childBridgeAdaptor(); - vm.expectRevert(InvalidData.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidData.selector, "Unsupported action signature")); axelarAdaptor.execute(commandId, CHILD_CHAIN_NAME, sourceAddress, data); } diff --git a/test/unit/child/ChildERC20Bridge.t.sol b/test/unit/child/ChildERC20Bridge.t.sol index 7bb37a26..72694bca 100644 --- a/test/unit/child/ChildERC20Bridge.t.sol +++ b/test/unit/child/ChildERC20Bridge.t.sol @@ -168,7 +168,7 @@ contract ChildERC20BridgeUnitTest is Test, IChildERC20BridgeEvents, IChildERC20B function test_RevertIf_onMessageReceiveCalledWithDataLengthZero() public { bytes memory data = ""; - vm.expectRevert(InvalidData.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidData.selector, "Data too short")); childBridge.onMessageReceive(ROOT_CHAIN_NAME, ROOT_BRIDGE_ADAPTOR, data); } @@ -176,7 +176,7 @@ contract ChildERC20BridgeUnitTest is Test, IChildERC20BridgeEvents, IChildERC20B bytes memory data = abi.encode("FAKEDATA", address(rootToken), rootToken.name(), rootToken.symbol(), rootToken.decimals()); - vm.expectRevert(InvalidData.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidData.selector, "Unsupported action signature")); childBridge.onMessageReceive(ROOT_CHAIN_NAME, ROOT_BRIDGE_ADAPTOR, data); } diff --git a/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol b/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol index 5ea3c0e4..be1999cd 100644 --- a/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol +++ b/test/unit/root/withdrawals/RootERC20BridgeWithdraw.t.sol @@ -84,14 +84,16 @@ contract RootERC20BridgeWithdrawUnitTest is Test, IRootERC20BridgeEvents, IRootE bytes memory data; vm.prank(address(mockAxelarAdaptor)); - vm.expectRevert(InvalidData.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidData.selector, "Data too short")); + rootBridge.onMessageReceive(CHILD_CHAIN_NAME, CHILD_BRIDGE_ADAPTOR_STRING, data); } function test_RevertsIf_OnMessageReceiveWithInvalidSignature() public { bytes memory data = abi.encode(keccak256("RANDOM"), IMX_TOKEN, address(this), address(this), withdrawAmount); + vm.prank(address(mockAxelarAdaptor)); - vm.expectRevert(InvalidData.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidData.selector, "Unsupported action signature")); rootBridge.onMessageReceive(CHILD_CHAIN_NAME, CHILD_BRIDGE_ADAPTOR_STRING, data); }