feat(ibc): validate remoteAddr string before making outbound connection

Agoric · Apr 25, 2024 · d2257ef · d2257ef
1 parent d077d70
commit d2257ef
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 6 deletions.
diff --git a/packages/network/src/network.js b/packages/network/src/network.js
@@ -497,11 +497,15 @@ const preparePort = (zone, powers) => {
           makeIncapable()
         ),
       ) {
-        const { revoked, localAddr, protocolImpl } = this.state;
+        const { revoked, localAddr, protocolImpl, protocolHandler } =
+          this.state;
 
         !revoked || Fail`Port ${localAddr} is revoked`;
         /** @type {Endpoint} */
         const dst = harden(remotePort);
+        await E(
+          /** @type {Remote<Required<ProtocolHandler>>} */ (protocolHandler),
+        ).validateRemoteAddr(dst);
         return watch(
           E(protocolImpl).outbound(this.facets.port, dst, connectionHandler),
           this.facets.portConnectWatcher,

diff --git a/packages/network/src/types.js b/packages/network/src/types.js
@@ -163,6 +163,8 @@
  *   p: Remote<ProtocolHandler>,
  * ) => PromiseVow<void>} onRevoke
  *   The port is being completely destroyed
+ * @property {(remoteAddr: string) => Promise<boolean>} validateRemoteAddr
+ *   Function to validate a remoteAddr is valid before attempting to connect to it.
  *
  * @typedef {object} InboundAttempt An inbound connection attempt
  * @property {(desc: AttemptDescription) => PromiseVow<Connection>} accept

diff --git a/packages/orchestration/test/utils/address.test.js b/packages/orchestration/test/utils/address.test.js
@@ -1,5 +1,5 @@
 import test from '@endo/ses-ava/prepare-endo.js';
-import { decodeRemoteIbcAddress } from '@agoric/vats/tools/ibc-utils.js';
+import { validateRemoteIbcAddress } from '@agoric/vats/tools/ibc-utils.js';
 import {
   makeICAChannelAddress,
   makeICQChannelAddress,
@@ -88,7 +88,7 @@ test('makeICQChannelAddress', t => {
   );
   t.throws(
     () =>
-      decodeRemoteIbcAddress(
+      validateRemoteIbcAddress(
         makeICQChannelAddress('connection-0', {
           version: 'ic/q-/2',
         }),
@@ -97,6 +97,6 @@ test('makeICQChannelAddress', t => {
       message:
         /must be '\(\/ibc-hop\/CONNECTION\)\*\/ibc-port\/PORT\/\(ordered\|unordered\)\/VERSION'/,
     },
-    'makeICQChannelAddress not hardened against malformed version. use `decodeRemoteIbcAddress` to detect this',
+    'makeICQChannelAddress not hardened against malformed version. use `validateRemoteIbcAddress` to detect this, or expect IBC ProtocolImpl to throw',
   );
 });
diff --git a/packages/vats/src/ibc.js b/packages/vats/src/ibc.js
@@ -12,6 +12,7 @@ import {
   decodeRemoteIbcAddress,
   encodeLocalIbcAddress,
   encodeRemoteIbcAddress,
+  validateRemoteIbcAddress,
 } from '../tools/ibc-utils.js';
 
 /** @import {LocalIbcAddress, RemoteIbcAddress} from '../tools/ibc-utils.js' */
@@ -243,6 +244,13 @@ export const prepareIBCProtocol = (zone, powers) => {
           this.state.lastPortID += 1n;
           return `port-${this.state.lastPortID}`;
         },
+        /**
+         * @param {string} remoteAddr
+         * @returns {Promise<boolean>}
+         */
+        async validateRemoteAddr(remoteAddr) {
+          return validateRemoteIbcAddress(remoteAddr);
+        },
         /** @type {Required<ProtocolHandler>['onBind']} */
         async onBind(_port, localAddr) {
           const { util } = this.facets;

diff --git a/packages/vats/test/test-network.js b/packages/vats/test/test-network.js
@@ -152,6 +152,20 @@ test('network - ibc', async t => {
 
   await testEcho();
 
+  const testInvalidRemoteAddr = async () => {
+    await E(p).addListener(makeIBCListener({ publisher }));
+    t.log('Connecting to a invalid remoteAddr throws');
+    await t.throwsAsync(
+      when(E(p).connect('/ibc-port/port-1/unordered/foo/f/f/f/f')),
+      {
+        message:
+          /must be '\(\/ibc-hop\/CONNECTION\)\*\/ibc-port\/PORT\/\(ordered\|unordered\)\/VERSION'/,
+      },
+    );
+  };
+
+  await testInvalidRemoteAddr();
+
   const testIBCOutbound = async () => {
     t.log('Connecting to a Remote Port');
     const [hopName, portName, version] = ['connection-11', 'port-98', 'bar'];

diff --git a/packages/vats/tools/ibc-utils.js b/packages/vats/tools/ibc-utils.js
@@ -6,15 +6,41 @@ harden(REMOTE_ADDR_RE);
 export const LOCAL_ADDR_RE = /^\/ibc-port\/(?<portID>[-a-zA-Z0-9._+#[\]<>]+)$/;
 /** @typedef {`/ibc-port/${string}`} LocalIbcAddress */
 
-/** @param {string} remoteAddr */
-export const decodeRemoteIbcAddress = remoteAddr => {
+/**
+ * @overload
+ * @param {string} remoteAddr
+ * @param {undefined | false} [returnMatch]
+ * @returns {boolean}
+ */
+/**
+ * @overload
+ * @param {string} remoteAddr
+ * @param {true} returnMatch
+ * @returns {RegExpMatchArray}
+ */
+/**
+ * Validates a remote IBC address format and returns true if the address is
+ * valid.
+ *
+ * @param {string} remoteAddr
+ * @param {boolean} [returnMatch]
+ */
+export const validateRemoteIbcAddress = (remoteAddr, returnMatch = false) => {
   const match = remoteAddr.match(REMOTE_ADDR_RE);
   // .groups is to inform TS https://github.com/microsoft/TypeScript/issues/32098
   if (!(match && match.groups)) {
     throw TypeError(
       `Remote address ${remoteAddr} must be '(/ibc-hop/CONNECTION)*/ibc-port/PORT/(ordered|unordered)/VERSION'`,
     );
   }
+  return returnMatch ? match : true;
+};
+
+/** @param {string} remoteAddr */
+export const decodeRemoteIbcAddress = remoteAddr => {
+  const match = validateRemoteIbcAddress(remoteAddr, true);
+  if (!match.groups)
+    throw Error('Unexpected error, validateRemoteIbcAddress should throw.');
 
   /** @type {import('../src/types.js').IBCConnectionID[]} */
   const hops = [];