From 19a3a30dc0d6f29a2e02b0d2665008937cf1ea9f Mon Sep 17 00:00:00 2001
From: w-sss <1598099293@qq.com>
Date: Sun, 29 Mar 2026 23:27:54 +0800
Subject: [PATCH] fix(matrix): correct DM classification without trusting
 remote user's is_direct flag

Problem: Matrix DM classification logic had security vulnerabilities:
1. Unreachable code branch when is_direct flag was absent
2. When is_direct: true, skipped 2-member check (shared rooms misclassified as DMs)
3. **CWE-285: Improper Authorization** - trusted remote user's is_direct flag

Security Issues:
- Remote attacker could set is_direct=true on their membership to force DM classification
- Remote attacker could set is_direct=false to bypass DM-only restrictions
- Both could lead to policy bypass (DM allowlist/pairing checks)

Fix:
- hasDirectMatrixMemberFlag() returns boolean | null for local user only
- isStrictDirectMembership() only trusts local user's is_direct (selfUserId)
- Removed directViaSender lookups entirely (do not trust remote-controlled data)
- Falls back to strict 2-member check when is_direct is false/null

Key Insights:
- In Matrix, m.room.member.content.is_direct is set by each member themselves
- Only trust signals the bot controls (local user's membership state)
- 2-member check remains as safe fallback that cannot be manipulated

Closes #56599
---
 extensions/matrix/src/matrix/direct-room.ts   | 40 ++++++++++++++++---
 .../matrix/src/matrix/monitor/direct.ts       | 13 +++++-
 2 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/extensions/matrix/src/matrix/direct-room.ts b/extensions/matrix/src/matrix/direct-room.ts
index cc11671c162..a2d48524191 100644
--- a/extensions/matrix/src/matrix/direct-room.ts
+++ b/extensions/matrix/src/matrix/direct-room.ts
@@ -21,10 +21,28 @@ export function isStrictDirectMembership(params: {
   selfUserId?: string | null;
   remoteUserId?: string | null;
   joinedMembers?: readonly string[] | null;
+  isDirectFlag?: boolean | null;
 }): boolean {
   const selfUserId = trimMaybeString(params.selfUserId);
   const remoteUserId = trimMaybeString(params.remoteUserId);
   const joinedMembers = params.joinedMembers ?? [];
+
+  // Only trust is_direct from local user's membership state (selfUserId).
+  // Remote user's is_direct is NOT trusted (CWE-285: authorization bypass).
+
+  // When local user's is_direct=true, verify it's a true 2-person DM
+  if (params.isDirectFlag === true) {
+    return Boolean(
+      selfUserId &&
+      remoteUserId &&
+      joinedMembers.length === 2 &&
+      joinedMembers.includes(selfUserId) &&
+      joinedMembers.includes(remoteUserId),
+    );
+  }
+
+  // When is_direct=false or null, fall back to strict 2-member check
+  // This prevents attackers from forcing non-DM classification with is_direct=false
   return Boolean(
     selfUserId &&
     remoteUserId &&
@@ -49,16 +67,25 @@ export async function hasDirectMatrixMemberFlag(
   client: MatrixClient,
   roomId: string,
   userId?: string | null,
-): Promise<boolean> {
+): Promise<boolean | null> {
   const normalizedUserId = trimMaybeString(userId);
   if (!normalizedUserId) {
-    return false;
+    return null;
   }
   try {
     const state = await client.getRoomStateEvent(roomId, "m.room.member", normalizedUserId);
-    return state?.is_direct === true;
+    // Return true if is_direct is explicitly true, false if explicitly false, null if absent
+    if (state?.is_direct === true) {
+      return true;
+    }
+    if (state?.is_direct === false) {
+      return false;
+    }
+    // is_direct field is absent from the membership event
+    return null;
   } catch {
-    return false;
+    // API/network error - treat as unavailable
+    return null;
   }
 }
 
@@ -95,8 +122,9 @@ export async function inspectMatrixDirectRoomEvidence(params: {
     joinedMembers,
     strict,
     viaMemberState:
-      (await hasDirectMatrixMemberFlag(params.client, params.roomId, params.remoteUserId)) ||
-      (await hasDirectMatrixMemberFlag(params.client, params.roomId, selfUserId)),
+      (await hasDirectMatrixMemberFlag(params.client, params.roomId, params.remoteUserId)) ===
+        true ||
+      (await hasDirectMatrixMemberFlag(params.client, params.roomId, selfUserId)) === true,
   };
 }
 
diff --git a/extensions/matrix/src/matrix/monitor/direct.ts b/extensions/matrix/src/matrix/monitor/direct.ts
index d976a794c81..822cce66755 100644
--- a/extensions/matrix/src/matrix/monitor/direct.ts
+++ b/extensions/matrix/src/matrix/monitor/direct.ts
@@ -82,10 +82,10 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr
   const resolveDirectMemberFlag = async (
     roomId: string,
     userId?: string | null,
-  ): Promise<boolean> => {
+  ): Promise<boolean | null> => {
     const normalizedUserId = userId?.trim();
     if (!normalizedUserId) {
-      return false;
+      return null;
     }
     const cacheKey = `${roomId}\n${normalizedUserId}`;
     const cached = directMemberFlagCache.get(cacheKey);
@@ -113,10 +113,19 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr
       const { roomId, senderId } = params;
       const selfUserId = params.selfUserId ?? (await ensureSelfUserId());
       const joinedMembers = await resolveJoinedMembers(roomId);
+
+      // Check is_direct flag from local user's membership state only.
+      // Do NOT trust remote sender's is_direct (CWE-285: improper authorization).
+      // In Matrix, m.room.member.content.is_direct is set by each member themselves,
+      // so a malicious remote user could manipulate it to bypass DM/room policies.
+      const directViaSelf = await resolveDirectMemberFlag(roomId, selfUserId);
+      const isDirectFlag: boolean | null = directViaSelf;
+
       const strictDirectMembership = isStrictDirectMembership({
         selfUserId,
         remoteUserId: senderId,
         joinedMembers,
+        isDirectFlag,
       });
 
       try {