Merge pull request #495 from xmtp/rygine/filter-invalid-topics

filter out conversations with invalid topics

src/Invitation.ts

@@ -47,7 +47,7 @@ export class InvitationV1 implements invitation.InvitationV1 {
         .replace(/=*$/g, '')
         // Replace slashes with dashes so that the topic is still easily split by /
         // We do not treat this as needing to be valid Base64 anywhere
-        .replace('/', '-')
+        .replace(/\//g, '-')
     )
     const keyMaterial = crypto.getRandomValues(new Uint8Array(32))
 

src/conversations/Conversations.ts

@@ -12,6 +12,7 @@ import {
   buildUserIntroTopic,
   buildUserInviteTopic,
   dateToNs,
+  isValidTopic,
   nsToDate,
 } from '../utils'
 import { PublicKeyBundle } from '../crypto'
@@ -81,14 +82,14 @@ export default class Conversations<ContentTypes = any> {
       })
 
       await this.client.keystore.saveV1Conversations({
-        conversations: Array.from(seenPeers).map(
-          ([peerAddress, createdAt]) => ({
+        conversations: Array.from(seenPeers)
+          .map(([peerAddress, createdAt]) => ({
             peerAddress,
             createdNs: dateToNs(createdAt),
             topic: buildDirectMessageTopic(peerAddress, this.client.address),
             context: undefined,
-          })
-        ),
+          }))
+          .filter((c) => isValidTopic(c.topic)),
       })
 
       return (
@@ -158,11 +159,13 @@ export default class Conversations<ContentTypes = any> {
     shouldThrow = false
   ): Promise<ConversationV2<ContentTypes>[]> {
     const { responses } = await this.client.keystore.saveInvites({
-      requests: envelopes.map((env) => ({
-        payload: env.message as Uint8Array,
-        timestampNs: Long.fromString(env.timestampNs as string),
-        contentTopic: env.contentTopic as string,
-      })),
+      requests: envelopes
+        .map((env) => ({
+          payload: env.message as Uint8Array,
+          timestampNs: Long.fromString(env.timestampNs as string),
+          contentTopic: env.contentTopic as string,
+        }))
+        .filter((req) => isValidTopic(req.contentTopic)),
     })
 
     const out: ConversationV2<ContentTypes>[] = []

src/utils/topic.ts

@@ -35,3 +35,18 @@ export const buildUserPrivateStoreTopic = (addrPrefixedKey: string): string => {
   // e.g. "0x1111111111222222222233333333334444444444/key_bundle"
   return buildContentTopic(`privatestore-${addrPrefixedKey}`)
 }
+
+// validate that a topic only contains ASCII characters 33-127
+export const isValidTopic = (topic: string): boolean => {
+  // eslint-disable-next-line no-control-regex
+  const regex = /^[\x21-\x7F]+$/
+  const index = topic.indexOf('0/')
+  if (index !== -1) {
+    const unwrappedTopic = topic.substring(
+      index + 2,
+      topic.lastIndexOf('/proto')
+    )
+    return regex.test(unwrappedTopic)
+  }
+  return false
+}

test/utils/topic.test.ts

@@ -0,0 +1,48 @@
+import {
+  buildContentTopic,
+  buildDirectMessageTopicV2,
+  isValidTopic,
+} from '../../src/utils/topic'
+import crypto from '../../src/crypto/crypto'
+
+describe('topic utils', () => {
+  describe('isValidTopic', () => {
+    it('validates topics correctly', () => {
+      expect(isValidTopic(buildContentTopic('foo'))).toBe(true)
+      expect(isValidTopic(buildContentTopic('123'))).toBe(true)
+      expect(isValidTopic(buildContentTopic('bar987'))).toBe(true)
+      expect(isValidTopic(buildContentTopic('*&+-)'))).toBe(true)
+      expect(isValidTopic(buildContentTopic('%#@='))).toBe(true)
+      expect(isValidTopic(buildContentTopic('<;.">'))).toBe(true)
+      expect(isValidTopic(buildContentTopic(String.fromCharCode(33)))).toBe(
+        true
+      )
+      expect(isValidTopic(buildContentTopic('∫ß'))).toBe(false)
+      expect(isValidTopic(buildContentTopic('\xA9'))).toBe(false)
+      expect(isValidTopic(buildContentTopic('\u2665'))).toBe(false)
+      expect(isValidTopic(buildContentTopic(String.fromCharCode(1)))).toBe(
+        false
+      )
+      expect(isValidTopic(buildContentTopic(String.fromCharCode(23)))).toBe(
+        false
+      )
+    })
+
+    it('validates random topics correctly', () => {
+      const topics = Array.from({ length: 100 }).map(() =>
+        buildDirectMessageTopicV2(
+          Buffer.from(crypto.getRandomValues(new Uint8Array(32)))
+            .toString('base64')
+            .replace(/=*$/g, '')
+            // Replace slashes with dashes so that the topic is still easily split by /
+            // We do not treat this as needing to be valid Base64 anywhere
+            .replace(/\//g, '-')
+        )
+      )
+
+      topics.forEach((topic) => {
+        expect(isValidTopic(topic)).toBe(true)
+      })
+    })
+  })
+})