fix writeTimeoutPacket vuln

open-ibc · Apr 11, 2024 · 5a8010a · 5a8010a
1 parent 76c1190
commit 5a8010a
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 9 deletions.
diff --git a/contracts/core/Dispatcher.sol b/contracts/core/Dispatcher.sol
@@ -474,7 +474,11 @@ contract Dispatcher is OwnableUpgradeable, UUPSUpgradeable, IDispatcher {
     /**
      * Generate a timeout packet for the given packet
      */
-    function writeTimeoutPacket(IbcPacket calldata packet) external {
+    function writeTimeoutPacket(IbcPacket calldata packet, Ics23Proof calldata proof) external {
+        _lightClient.verifyMembership(
+            proof, Ibc.packetCommitmentProofKey(packet), abi.encode(Ibc.packetCommitmentProofValue(packet))
+        );
+
         address receiver = _getAddressFromPort(packet.src.portId);
         // verify packet does not have a receipt
         bool hasReceipt = _recvPacketReceipt[receiver][packet.dest.channelId][packet.sequence];

diff --git a/contracts/interfaces/IDispatcher.sol b/contracts/interfaces/IDispatcher.sol
@@ -89,6 +89,8 @@ interface IDispatcher is IbcDispatcher, IbcEventsEmitter {
 
     function timeout(IbcPacket calldata packet, Ics23Proof calldata proof) external;
 
+    function writeTimeoutPacket(IbcPacket calldata packet, Ics23Proof calldata proof) external;
+
     function recvPacket(IbcPacket calldata packet, Ics23Proof calldata proof) external;
 
     function getOptimisticConsensusState(uint256 height)

diff --git a/test/Dispatcher.proof.t.sol b/test/Dispatcher.proof.t.sol
@@ -145,7 +145,7 @@ contract DispatcherIbcWithRealProofs is DispatcherIbcWithRealProofsSuite {
         (dispatcherProxy, dispatcherImplementation) = deployDispatcherProxyAndImpl(portPrefix1, consensusStateManager);
 
         address targetMarsAddress = 0x71C95911E9a5D330f4D621842EC243EE1343292e;
-        deployCodeTo("Mars.sol", abi.encode(address(dispatcherProxy)), targetMarsAddress);
+        deployCodeTo("Mars.sol:Mars", abi.encode(address(dispatcherProxy)), targetMarsAddress);
         mars = Mars(payable(targetMarsAddress));
 
         portId1 = "polyibc.eth1.71C95911E9a5D330f4D621842EC243EE1343292e";

diff --git a/test/Dispatcher.t.sol b/test/Dispatcher.t.sol
@@ -336,7 +336,7 @@ contract PacketSenderTestBase is ChannelOpenTestBaseSetup {
 
 // Test Chains B receives a packet from Chain A
 contract DispatcherRecvPacketTestSuite is ChannelOpenTestBaseSetup {
-    IbcEndpoint src = IbcEndpoint("polyibc.bsc.9876543210", "channel-99");
+    IbcEndpoint src = IbcEndpoint("polyibc.bsc.58b604DB8886656695442374D8E940D814F2eDd4", "channel-99");
     IbcEndpoint dest;
     bytes payload = bytes("msgPayload");
     bytes appAck = abi.encodePacked('{ "account": "account", "reply": "got the message" }');
@@ -387,6 +387,24 @@ contract DispatcherRecvPacketTestSuite is ChannelOpenTestBaseSetup {
         dispatcherProxy.recvPacket(IbcPacket(src, dest, 3, payload, ZERO_HEIGHT, maxTimeout), validProof);
     }
 
+    // Can't spoof a timeed out packet
+    function test_writeTimeoutPacket_cannot_Spoof_Packet() public {
+        IbcPacket memory spoofedPacket = IbcPacket(src, dest, 1, payload, ZERO_HEIGHT, 1);
+        spoofedPacket.timeoutTimestamp = 1; // Set really low timeout timestamp
+        vm.mockCallRevert(
+            address(dummyConsStateManager),
+            abi.encodeWithSelector(
+                DummyLightClient.verifyMembership.selector,
+                validProof,
+                Ibc.packetCommitmentProofKey(spoofedPacket),
+                abi.encode(Ibc.packetCommitmentProofValue(spoofedPacket))
+            ),
+            abi.encodeWithSelector(ProofVerifier.InvalidProofValue.selector)
+        );
+        vm.expectRevert(abi.encodeWithSelector(ProofVerifier.InvalidProofValue.selector));
+        dispatcherProxy.writeTimeoutPacket(spoofedPacket, validProof);
+    }
+
     // TODO: add tests for unordered channel, wrong port, and invalid proof
 }
 

diff --git a/test/upgradeableProxy/upgrades/DispatcherV2.sol b/test/upgradeableProxy/upgrades/DispatcherV2.sol
@@ -473,15 +473,15 @@ contract DispatcherV2 is OwnableUpgradeable, UUPSUpgradeable, IDispatcher {
     /**
      * Generate a timeout packet for the given packet
      */
-    function writeTimeoutPacket(IbcPacket calldata packet) external {
-        // verify `receiver` is the original packet sender
-        // if (!portIdAddressMatch(receiver, packet.src.portId)) {
-        //     revert IBCErrors.receiverNotIntendedPacketDestination();
-        // }
+    function writeTimeoutPacket(IbcPacket calldata packet, Ics23Proof calldata proof) external {
+        _lightClient.verifyMembership(
+            proof, Ibc.packetCommitmentProofKey(packet), abi.encode(Ibc.packetCommitmentProofValue(packet))
+        );
 
-        address receiver = _getAddressFromPort(packet.dest.portId);
+        address receiver = _getAddressFromPort(packet.src.portId);
         // verify packet does not have a receipt
         bool hasReceipt = _recvPacketReceipt[receiver][packet.dest.channelId][packet.sequence];
+
         if (hasReceipt) {
             revert IBCErrors.packetReceiptAlreadyExists();
         }