[lib] Validate IDs in DM operations

Summary: We should check whether the IDs are thick - it protects us against an attacker who could try to create operations referencing thin thread entities. https://linear.app/comm/issue/ENG-9826/validate-the-ids-from-the-dm-operations Depends on D13848 Test Plan: Tested a couple of scenarios: - sending a text message - changing thread settings - editing a message - reacting to a message - creating a sidebar In the cases where another message was a target, tested that it works for both text and edit thread settings messages. Reviewers: kamil, angelika Reviewed By: kamil Subscribers: ashoat Differential Revision: https://phab.comm.dev/D13858
CommE2E · Nov 8, 2024 · d8ed20c · d8ed20c
1 parent 8eec4d2
commit d8ed20c
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 50 deletions.
diff --git a/lib/types/dm-ops.js b/lib/types/dm-ops.js
@@ -25,7 +25,15 @@ import {
 } from './thread-types.js';
 import type { ClientUpdateInfo } from './update-types.js';
 import { values } from '../utils/objects.js';
-import { tColor, tShape, tString, tUserID } from '../utils/validation-utils.js';
+import {
+  tColor,
+  thickIDRegex,
+  tRegex,
+  tShape,
+  tString,
+  tUserID,
+  uuidRegex,
+} from '../utils/validation-utils.js';
 
 export const dmOperationTypes = Object.freeze({
   CREATE_THREAD: 'create_thread',
@@ -48,6 +56,14 @@ export const dmOperationTypes = Object.freeze({
 });
 export type DMOperationType = $Values<typeof dmOperationTypes>;
 
+const tThickID = tRegex(thickIDRegex);
+
+// In CHANGE_THREAD_SETTINGS operation we're generating message IDs
+// based on the prefix that is tThickID. A message with the generated ID
+// can be used as a target message of the edit, reaction, or a sidebar.
+const thickTargetMessageIDRegex = new RegExp(`^${uuidRegex}(?:/\\w+)?$`);
+const tThickTargetMessageID = tRegex(thickTargetMessageIDRegex);
+
 type MemberIDWithSubscription = {
   +id: string,
   +subscription: ThreadSubscription,
@@ -78,20 +94,20 @@ export type CreateThickRawThreadInfoInput = {
 };
 export const createThickRawThreadInfoInputValidator: TInterface<CreateThickRawThreadInfoInput> =
   tShape<CreateThickRawThreadInfoInput>({
-    threadID: t.String,
+    threadID: tThickID,
     threadType: thickThreadTypeValidator,
     creationTime: t.Number,
-    parentThreadID: t.maybe(t.String),
+    parentThreadID: t.maybe(tThickID),
     allMemberIDsWithSubscriptions: t.list(memberIDWithSubscriptionValidator),
-    roleID: t.String,
+    roleID: tThickID,
     unread: t.Boolean,
     timestamps: threadTimestampsValidator,
     name: t.maybe(t.String),
     avatar: t.maybe(clientAvatarValidator),
     description: t.maybe(t.String),
     color: t.maybe(t.String),
-    containingThreadID: t.maybe(t.String),
-    sourceMessageID: t.maybe(t.String),
+    containingThreadID: t.maybe(tThickID),
+    sourceMessageID: t.maybe(tThickTargetMessageID),
     repliesCount: t.maybe(t.Number),
     pinnedCount: t.maybe(t.Number),
   });
@@ -109,13 +125,13 @@ export type DMCreateThreadOperation = {
 export const dmCreateThreadOperationValidator: TInterface<DMCreateThreadOperation> =
   tShape<DMCreateThreadOperation>({
     type: tString(dmOperationTypes.CREATE_THREAD),
-    threadID: t.String,
+    threadID: tThickID,
     creatorID: tUserID,
     time: t.Number,
     threadType: t.enums.of(values(nonSidebarThickThreadTypes)),
     memberIDs: t.list(tUserID),
-    roleID: t.String,
-    newMessageID: t.String,
+    roleID: tThickID,
+    newMessageID: tThickID,
   });
 
 export type DMCreateSidebarOperation = {
@@ -133,15 +149,15 @@ export type DMCreateSidebarOperation = {
 export const dmCreateSidebarOperationValidator: TInterface<DMCreateSidebarOperation> =
   tShape<DMCreateSidebarOperation>({
     type: tString(dmOperationTypes.CREATE_SIDEBAR),
-    threadID: t.String,
+    threadID: tThickID,
     creatorID: tUserID,
     time: t.Number,
-    parentThreadID: t.String,
+    parentThreadID: tThickID,
     memberIDs: t.list(tUserID),
-    sourceMessageID: t.String,
-    roleID: t.String,
-    newSidebarSourceMessageID: t.String,
-    newCreateSidebarMessageID: t.String,
+    sourceMessageID: tThickTargetMessageID,
+    roleID: tThickID,
+    newSidebarSourceMessageID: tThickID,
+    newCreateSidebarMessageID: tThickID,
   });
 
 export type DMSendTextMessageOperation = {
@@ -155,10 +171,10 @@ export type DMSendTextMessageOperation = {
 export const dmSendTextMessageOperationValidator: TInterface<DMSendTextMessageOperation> =
   tShape<DMSendTextMessageOperation>({
     type: tString(dmOperationTypes.SEND_TEXT_MESSAGE),
-    threadID: t.String,
+    threadID: tThickID,
     creatorID: tUserID,
     time: t.Number,
-    messageID: t.String,
+    messageID: tThickID,
     text: t.String,
   });
 
@@ -173,10 +189,10 @@ export type DMSendMultimediaMessageOperation = {
 export const dmSendMultimediaMessageOperationValidator: TInterface<DMSendMultimediaMessageOperation> =
   tShape<DMSendMultimediaMessageOperation>({
     type: tString(dmOperationTypes.SEND_MULTIMEDIA_MESSAGE),
-    threadID: t.String,
+    threadID: tThickID,
     creatorID: tUserID,
     time: t.Number,
-    messageID: t.String,
+    messageID: tThickID,
     media: t.list(mediaValidator),
   });
 
@@ -193,11 +209,11 @@ export type DMSendReactionMessageOperation = {
 export const dmSendReactionMessageOperationValidator: TInterface<DMSendReactionMessageOperation> =
   tShape<DMSendReactionMessageOperation>({
     type: tString(dmOperationTypes.SEND_REACTION_MESSAGE),
-    threadID: t.String,
+    threadID: tThickID,
     creatorID: tUserID,
     time: t.Number,
-    messageID: t.String,
-    targetMessageID: t.String,
+    messageID: tThickID,
+    targetMessageID: tThickTargetMessageID,
     reaction: t.String,
     action: t.enums.of(['add_reaction', 'remove_reaction']),
   });
@@ -214,11 +230,11 @@ export type DMSendEditMessageOperation = {
 export const dmSendEditMessageOperationValidator: TInterface<DMSendEditMessageOperation> =
   tShape<DMSendEditMessageOperation>({
     type: tString(dmOperationTypes.SEND_EDIT_MESSAGE),
-    threadID: t.String,
+    threadID: tThickID,
     creatorID: tUserID,
     time: t.Number,
-    messageID: t.String,
-    targetMessageID: t.String,
+    messageID: tThickID,
+    targetMessageID: tThickTargetMessageID,
     text: t.String,
   });
 
@@ -242,8 +258,8 @@ export type DMAddMembersOperation = $ReadOnly<{
 export const dmAddMembersOperationValidator: TInterface<DMAddMembersOperation> =
   tShape<DMAddMembersOperation>({
     type: tString(dmOperationTypes.ADD_MEMBERS),
-    threadID: t.String,
-    messageID: t.String,
+    threadID: tThickID,
+    messageID: tThickID,
     ...dmAddMembersBaseValidatorShape,
   });
 
@@ -256,7 +272,7 @@ export type DMAddViewerToThreadMembersOperation = $ReadOnly<{
 export const dmAddViewerToThreadMembersValidator: TInterface<DMAddViewerToThreadMembersOperation> =
   tShape<DMAddViewerToThreadMembersOperation>({
     type: tString(dmOperationTypes.ADD_VIEWER_TO_THREAD_MEMBERS),
-    messageID: t.maybe(t.String),
+    messageID: t.maybe(tThickID),
     existingThreadDetails: createThickRawThreadInfoInputValidator,
     ...dmAddMembersBaseValidatorShape,
   });
@@ -273,7 +289,7 @@ export const dmJoinThreadOperationValidator: TInterface<DMJoinThreadOperation> =
     type: tString(dmOperationTypes.JOIN_THREAD),
     joinerID: tUserID,
     time: t.Number,
-    messageID: t.String,
+    messageID: tThickID,
     existingThreadDetails: createThickRawThreadInfoInputValidator,
   });
 
@@ -289,8 +305,8 @@ export const dmLeaveThreadOperationValidator: TInterface<DMLeaveThreadOperation>
     type: tString(dmOperationTypes.LEAVE_THREAD),
     editorID: tUserID,
     time: t.Number,
-    messageID: t.String,
-    threadID: t.String,
+    messageID: tThickID,
+    threadID: tThickID,
   });
 
 export type DMThreadSettingsChanges = {
@@ -311,7 +327,7 @@ export type DMChangeThreadSettingsOperation = $ReadOnly<{
 export const dmChangeThreadSettingsOperationValidator: TInterface<DMChangeThreadSettingsOperation> =
   tShape<DMChangeThreadSettingsOperation>({
     type: tString(dmOperationTypes.CHANGE_THREAD_SETTINGS),
-    threadID: t.String,
+    threadID: tThickID,
     editorID: tUserID,
     time: t.Number,
     changes: tShape({
@@ -320,7 +336,7 @@ export const dmChangeThreadSettingsOperationValidator: TInterface<DMChangeThread
       color: t.maybe(tColor),
       avatar: t.maybe(clientAvatarValidator),
     }),
-    messageIDsPrefix: t.String,
+    messageIDsPrefix: tThickID,
   });
 
 export type DMChangeThreadSubscriptionOperation = {
@@ -334,7 +350,7 @@ export const dmChangeThreadSubscriptionOperationValidator: TInterface<DMChangeTh
   tShape<DMChangeThreadSubscriptionOperation>({
     type: tString(dmOperationTypes.CHANGE_THREAD_SUBSCRIPTION),
     time: t.Number,
-    threadID: t.String,
+    threadID: tThickID,
     creatorID: tUserID,
     subscription: threadSubscriptionValidator,
   });
@@ -350,7 +366,7 @@ export const dmChangeThreadReadStatusOperationValidator: TInterface<DMChangeThre
   tShape<DMChangeThreadReadStatusOperation>({
     type: tString(dmOperationTypes.CHANGE_THREAD_READ_STATUS),
     time: t.Number,
-    threadID: t.String,
+    threadID: tThickID,
     creatorID: tUserID,
     unread: t.Boolean,
   });
@@ -372,13 +388,13 @@ export type DMCreateEntryOperation = {
 export const dmCreateEntryOperationValidator: TInterface<DMCreateEntryOperation> =
   tShape<DMCreateEntryOperation>({
     type: tString(dmOperationTypes.CREATE_ENTRY),
-    threadID: t.String,
+    threadID: tThickID,
     creatorID: tUserID,
     time: t.Number,
-    entryID: t.String,
+    entryID: tThickID,
     entryDate: t.String,
     text: t.String,
-    messageID: t.String,
+    messageID: tThickID,
   });
 
 export type DMDeleteEntryOperation = {
@@ -395,14 +411,14 @@ export type DMDeleteEntryOperation = {
 export const dmDeleteEntryOperationValidator: TInterface<DMDeleteEntryOperation> =
   tShape<DMDeleteEntryOperation>({
     type: tString(dmOperationTypes.DELETE_ENTRY),
-    threadID: t.String,
+    threadID: tThickID,
     creatorID: tUserID,
     time: t.Number,
     creationTime: t.Number,
-    entryID: t.String,
+    entryID: tThickID,
     entryDate: t.String,
     prevText: t.String,
-    messageID: t.String,
+    messageID: tThickID,
   });
 
 export type DMEditEntryOperation = {
@@ -419,14 +435,14 @@ export type DMEditEntryOperation = {
 export const dmEditEntryOperationValidator: TInterface<DMEditEntryOperation> =
   tShape<DMEditEntryOperation>({
     type: tString(dmOperationTypes.EDIT_ENTRY),
-    threadID: t.String,
+    threadID: tThickID,
     creatorID: tUserID,
     creationTime: t.Number,
     time: t.Number,
-    entryID: t.String,
+    entryID: tThickID,
     entryDate: t.String,
     text: t.String,
-    messageID: t.String,
+    messageID: tThickID,
   });
 
 export type DMUpdateRelationshipOperation = {
@@ -441,7 +457,7 @@ export type DMUpdateRelationshipOperation = {
 export const dmUpdateRelationshipOperationValidator: TInterface<DMUpdateRelationshipOperation> =
   tShape<DMUpdateRelationshipOperation>({
     type: tString(dmOperationTypes.UPDATE_RELATIONSHIP),
-    threadID: t.String,
+    threadID: tThickID,
     creatorID: tUserID,
     time: t.Number,
     operation: t.enums.of([
@@ -450,7 +466,7 @@ export const dmUpdateRelationshipOperationValidator: TInterface<DMUpdateRelation
       'farcaster_mutual',
     ]),
     targetUserID: tUserID,
-    messageID: t.String,
+    messageID: tThickID,
   });
 
 export type DMOperation =

diff --git a/lib/utils/validation-utils.js b/lib/utils/validation-utils.js
@@ -115,7 +115,7 @@ const idSchemaRegex = `(?:(?:[0-9]+|${uuidRegex})\\|)?(?:[0-9]+|${uuidRegex})`;
 const pendingSidebarURLPrefix = 'sidebar';
 const pendingThickSidebarURLPrefix = 'dm_sidebar';
 const pendingThreadIDRegex = `pending/(type[0-9]+/[0-9]+(\\+[0-9]+)*|(${pendingSidebarURLPrefix}|${pendingThickSidebarURLPrefix})/${idSchemaRegex})`;
-const thickThreadIDRegex: RegExp = new RegExp(`^${uuidRegex}$`);
+const thickIDRegex: RegExp = new RegExp(`^${uuidRegex}$`);
 
 const chatNameMaxLength = 191;
 const chatNameMinLength = 0;
@@ -147,11 +147,12 @@ export {
   tMediaMessageMedia,
   assertWithValidator,
   ashoatKeyserverID,
+  uuidRegex,
   idSchemaRegex,
   pendingSidebarURLPrefix,
   pendingThickSidebarURLPrefix,
   pendingThreadIDRegex,
-  thickThreadIDRegex,
+  thickIDRegex,
   validChatNameRegex,
   validChatNameRegexString,
   chatNameMaxLength,

diff --git a/web/redux/initial-state-gate.js b/web/redux/initial-state-gate.js
@@ -17,7 +17,7 @@ import { convertIDToNewSchema } from 'lib/utils/migration-utils.js';
 import { entries, values } from 'lib/utils/objects.js';
 import { useDispatch } from 'lib/utils/redux-utils.js';
 import { infoFromURL } from 'lib/utils/url-utils.js';
-import { thickThreadIDRegex } from 'lib/utils/validation-utils.js';
+import { thickIDRegex } from 'lib/utils/validation-utils.js';
 
 import {
   setInitialReduxState,
@@ -58,7 +58,7 @@ function InitialReduxStateGate(props: Props): React.Node {
       try {
         let urlInfo = infoFromURL(decodeURI(window.location.href));
         const isThickThreadOpen =
-          urlInfo.thread && thickThreadIDRegex.test(urlInfo.thread);
+          urlInfo.thread && thickIDRegex.test(urlInfo.thread);
         // Handle older links
         if (urlInfo.thread && !isThickThreadOpen) {
           urlInfo = {