1. 🟠 DM/room policy bypass by trusting sender-controlled m.room.member.is_direct

Description

createDirectRoomTracker().isDirectMessage() treats a room as a DM when either the remote sender’s membership state (m.room.member for senderId) or the local user’s membership state has is_direct=true.

In Matrix, m.room.member.content.is_direct is not an authoritative protocol-level signal for the other party; it is a per-user hint set by the member themselves. This means a remote user can set their own membership event field is_direct=true and cause the application to classify a 2-person room as a DM for the bot.

Impact (grounded in current code paths):

• DM vs room classification directly affects enforcement of groupPolicy and room allowlists in the inbound event handler.
• If a deployment restricts rooms (e.g., groupPolicy === "disabled" or allowlist mode), misclassification as a DM can allow messages/commands to be processed that would otherwise be dropped as a room message.

Vulnerable logic:

• Remote-controlled input: senderId (event sender) -> hasDirectMatrixMemberFlag(..., senderId) reads remote user’s m.room.member state.
• Decision: remote is_direct=true is considered “authoritative DM”, feeding isStrictDirectMembership() and later DM detection.

Vulnerable code:

const directViaSender = await resolveDirectMemberFlag(roomId, senderId);
...
const isDirectFlag: boolean | null =
  directViaSender === true || directViaSelf === true
    ? true
    : ...

Recommendation

Do not use the remote user’s m.room.member.is_direct to classify DMs for the local account.

Recommended approach:

• Treat DM as true only when indicated by local account data (m.direct) and/or the local user’s membership state m.room.member for selfUserId.
• Optionally keep the 2-member fallback as a non-authoritative heuristic only before seeding account data (as you already do), but do not allow remote is_direct to override policies.

Example fix:

const directViaSelf = await resolveDirectMemberFlag(roomId, selfUserId);
const isDirectFlag: boolean | null =
  directViaSelf === true ? true : directViaSelf === false ? false : null;

const strictDirectMembership = isStrictDirectMembership({
  selfUserId,
  remoteUserId: senderId,
  joinedMembers,
  isDirectFlag,
});

If you need “explicit DM” signals from the room, consider only signals the bot controls (its own account data / own membership state), not other members’ hints.

2. 🟠 DM detection bypass via attacker-controlled m.room.member.is_direct=false

Description

The DM/room classification logic now treats an explicit is_direct=false in a user's m.room.member state event as authoritative and immediately forces isStrictDirectMembership to return false.

In Matrix, the content of a member's own m.room.member event is (in practice) under that member's control when they join/update profile fields. A malicious sender in a 2-person room can therefore publish is_direct: false on their membership event, causing the bot to misclassify an otherwise strict 2-member DM as a group room.

This changes downstream authorization behavior in monitor/handler.ts:

• If the room is misclassified as isRoom=true, DM policy checks (e.g. pairing/allowlist) are skipped.
• The message will instead follow room policy (groupPolicy, per-room allowlists, etc.), which may be configured more permissively than DM policy.

Vulnerable logic:

• is_direct=false ⇒ immediate return false; (no fallback to 2-member strict check)

​// Priority 2: explicit is_direct=false means not a DM
if (params.isDirectFlag === false) {
  return false;
}

and the flag is derived from untrusted membership state:

const directViaSender = await resolveDirectMemberFlag(roomId, senderId);
...
const isDirectFlag: boolean | null =
  directViaSender === true || directViaSelf === true ? true
  : directViaSender === false || directViaSelf === false ? false
  : null;

Impact: authorization bypass where DM-only restrictions (e.g., pairing) can be avoided by forcing the message down the room-processing path.

Recommendation

Do not treat is_direct=false from m.room.member state as authoritative for security decisions.

Safer options:

1. Only treat is_direct=true as a positive signal, and treat false/missing as “unknown”, falling back to strict 2-member check:

​// Only accept explicit true as authoritative
if (params.isDirectFlag === true) {
  return selfUserId && remoteUserId && joinedMembers.length === 2 &&
    joinedMembers.includes(selfUserId) && joinedMembers.includes(remoteUserId);
}

​// For false/null: fall back to strict 2-member check
return selfUserId && remoteUserId && joinedMembers.length === 2 &&
  joinedMembers.includes(selfUserId) && joinedMembers.includes(remoteUserId);

2. Prefer a trusted DM signal (e.g., m.direct account data owned by the bot account) rather than remote-controlled membership content.

If you still want negative signals, require they come from bot-controlled sources (e.g., only self member event, or only after validating server-side semantics) and never use them to weaken authorization checks.

3. 🟡 DM access-control bypass via fail-open fallback when is_direct member state is unavailable

Description

The Matrix DM classification logic was changed to treat missing userId and getRoomStateEvent failures as null ("unavailable") instead of false. In createDirectRoomTracker().isDirectMessage, when is_direct is unavailable it falls back to a permissive heuristic (exactly 2 joined members) and, before the DM cache is seeded, returns true (DM) even without authoritative signals.

This can misclassify a non-DM 2-person room as a DM if an attacker can cause:

• getRoomStateEvent(roomId, "m.room.member", userId) to fail (permissions, rate limiting, transient network issues)
• userId lookups to be missing/invalid

When misclassified as a DM, downstream policy decisions treat the room as a direct message. For example, monitor/handler.ts gates behavior differently for isDirectMessage vs isRoom (DM policy/pairing vs group allowlist/disabled), so this can enable DM-only flows or bypass group restrictions for a 2-person room.

Vulnerable code (fail-open to heuristic when state flag unavailable):

const directViaSender = await resolveDirectMemberFlag(roomId, senderId);
const directViaSelf = await resolveDirectMemberFlag(roomId, selfUserId);
const isDirectFlag: boolean | null =
  directViaSender === true || directViaSelf === true
    ? true
    : directViaSender === false || directViaSelf === false
      ? false
      : null; // both null

const strictDirectMembership = isStrictDirectMembership({
  selfUserId,
  remoteUserId: senderId,
  joinedMembers,
  isDirectFlag,
});
...
if (strictDirectMembership) {
  ...
  if (!hasSeededDmCache) {
    return true; // accept 2-member fallback
  }
}

And the root cause for null on errors:

} catch {
  ​// API/network error - treat as unavailable
  return null;
}

Impact depends on deployment policy, but in configurations where DMs are allowed more broadly than rooms (e.g., groupPolicy disabled/allowlist while DM policy is open/pairing), an attacker can interact with the bot through a 2-person room by inducing is_direct lookup failures (especially during startup before m.direct cache seeds).

Recommendation

Fail closed for DM classification when authoritative signals are unavailable, or at least avoid using the 2-member heuristic in security-sensitive gating.

Options:

1. Treat getRoomStateEvent errors as explicit false (not DM) rather than null:

export async function hasDirectMatrixMemberFlag(...): Promise<boolean | null> {
  ...
  try {
    const state = await client.getRoomStateEvent(roomId, "m.room.member", normalizedUserId);
    if (state?.is_direct === true) return true;
    if (state?.is_direct === false) return false;
    return null; // only when field is absent
  } catch {
    return false; // fail closed on error
  }
}

2. Gate the fallback so it only applies when you have positive evidence the room is intended as a DM (e.g., m.direct mapping) rather than when state is unreachable. For example:

const canUseHeuristic = isDirectFlag === null && client.dms.isDm(roomId);
const strictDirectMembership = isStrictDirectMembership({ ..., isDirectFlag: canUseHeuristic ? null : false });

3. If the 2-member heuristic must remain, do not use it for authorization/policy decisions. Keep it only for UX hints/logging, and require m.direct or explicit is_direct:true for DM-only actions.

1. extensions/matrix/src/matrix/monitor/direct.ts, line 40 (link)

   Cache type not updated for boolean | null

   directMemberFlagCache is still typed with isDirect: boolean, but hasDirectMatrixMemberFlag now returns Promise<boolean | null>. When a null result is stored on line 97, TypeScript strict-null checking will reject the assignment as null is not assignable to boolean. The cache type needs to be updated to match the new return type.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: extensions/matrix/src/matrix/monitor/direct.ts
   Line: 40
   
   Comment:
   **Cache type not updated for `boolean | null`**
   
   `directMemberFlagCache` is still typed with `isDirect: boolean`, but `hasDirectMatrixMemberFlag` now returns `Promise<boolean | null>`. When a `null` result is stored on line 97, TypeScript strict-null checking will reject the assignment as `null` is not assignable to `boolean`. The cache type needs to be updated to match the new return type.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

2. extensions/matrix/src/matrix/direct-room.ts, line 111-115 (link)

   Security fix doesn't cover isStrictDirectRoom / inspectMatrixDirectRoomEvidence

   The inspectMatrixDirectRoomEvidence function computes strict by calling isStrictDirectMembership without passing isDirectFlag, so the "explicit is_direct: false is authoritative" fix is completely absent on this code path. All callers of isStrictDirectRoom would still classify a 2-member room as a DM even when both membership events carry is_direct: false.

   Active callers include:

   • extensions/matrix/src/matrix/send/targets.ts:58 — target resolution
   • extensions/matrix/src/matrix/monitor/verification-events.ts:232, 453, 470, 530 — verification routing
   • extensions/matrix/src/matrix/direct-management.ts:86 — DM candidate classification

   The is_direct flag from both members should be fetched and passed into isStrictDirectMembership here, mirroring what was done in monitor/direct.ts.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: extensions/matrix/src/matrix/direct-room.ts
   Line: 111-115
   
   Comment:
   **Security fix doesn't cover `isStrictDirectRoom` / `inspectMatrixDirectRoomEvidence`**
   
   The `inspectMatrixDirectRoomEvidence` function computes `strict` by calling `isStrictDirectMembership` **without** passing `isDirectFlag`, so the "explicit `is_direct: false` is authoritative" fix is completely absent on this code path. All callers of `isStrictDirectRoom` would still classify a 2-member room as a DM even when both membership events carry `is_direct: false`.
   
   Active callers include:
   - `extensions/matrix/src/matrix/send/targets.ts:58` — target resolution
   - `extensions/matrix/src/matrix/monitor/verification-events.ts:232, 453, 470, 530` — verification routing
   - `extensions/matrix/src/matrix/direct-management.ts:86` — DM candidate classification
   
   The `is_direct` flag from both members should be fetched and passed into `isStrictDirectMembership` here, mirroring what was done in `monitor/direct.ts`.
   
   How can I resolve this? If you propose a fix, please make it concise.

3. extensions/matrix/src/matrix/monitor/direct.ts, line 152-154 (link)

   Redundant resolveDirectMemberFlag calls recompute already-available values

   directViaSender and directViaSelf were already resolved and stored on lines 118–119. Calling resolveDirectMemberFlag again here is redundant (even if cached) and inconsistent with the === true pattern used for the viaMemberState field in direct-room.ts. Since isDirectFlag !== false at this point (due to strictDirectMembership === true), the || operator works correctly in practice, but reusing the earlier values is cleaner and avoids any ambiguity:

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: extensions/matrix/src/matrix/monitor/direct.ts
   Line: 152-154
   
   Comment:
   **Redundant `resolveDirectMemberFlag` calls recompute already-available values**
   
   `directViaSender` and `directViaSelf` were already resolved and stored on lines 118–119. Calling `resolveDirectMemberFlag` again here is redundant (even if cached) and inconsistent with the `=== true` pattern used for the `viaMemberState` field in `direct-room.ts`. Since `isDirectFlag !== false` at this point (due to `strictDirectMembership === true`), the `||` operator works correctly in practice, but reusing the earlier values is cleaner and avoids any ambiguity:
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

   _{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

1. 🟠 DM classification trusts remote user's is_direct membership flag (authorization bypass)

Description

In createDirectRoomTracker().isDirectMessage(), the DM decision can be made based on the remote sender's m.room.member.content.is_direct flag:

• Matrix allows each member to set their own membership content.
• The code checks (await resolveDirectMemberFlag(roomId, senderId)) and treats any truthy value as DM evidence.
• A malicious remote user can set is_direct: true on their own membership state in a 2-member room, causing the system to classify the room as a DM even if the local user did not mark it as such.

This contradicts the comment in the same file (“Do NOT trust remote sender's is_direct”) and can enable DM-only policies/behaviors to be applied inappropriately (CWE-285).

Vulnerable code:

const directViaState =
  (await resolveDirectMemberFlag(roomId, senderId)) ||
  (await resolveDirectMemberFlag(roomId, selfUserId));
if (directViaState) {
  ...
  return true;
}

Recommendation

Only consider is_direct from the local user's membership state (self) when using it as DM evidence, and do not OR-in the remote sender's flag.

For example:

​// Only trust local user's is_direct
const directViaState = (await resolveDirectMemberFlag(roomId, selfUserId)) === true;
if (directViaState) {
  log(`matrix: dm detected via member state room=${roomId}`);
  return true;
}

If you still want a 2-member fallback, keep it separate and gated (e.g., only before cache seed), but do not use the remote user's is_direct to elevate DM classification.