T09 · Insecure Skill Coding Practices
Error
- Location
- src/voice-connection.ts:886
- Finding
- Speaker Authorization Fails Open When the Primary User Cannot Be Resolved<![CDATA[ ## Vulnerability Details **File Location**: `src/voice-connection.ts:886-900` and `src/voice-connection.ts:947-958` **Vulnerability Type**: Fail-open access control **Risk Level**: High ### Vulnerable Code ```ts private resolvePrimaryUser(session: VoiceSession): void { if (!this.config.primaryUser) return; const id = this.resolveUserId(session, this.config.primaryUser); if (id) { session.primaryUserId = id; this.logger.info(`[discord-voice] Primary speaker resolved to userId=${id}`); } else { // If we can't resolve (name not in channel yet), we keep primaryUserId unset. // The user can re-join / or say the switch command after they join. this.logger.warn(`[discord-voice] Could not resolve primaryUser="${this.config.primaryUser}" in this voice channel yet.`); } } ``` ```ts private isUserAllowed(session: VoiceSession, userId: string): boolean { // If primaryUser is set, default to listening ONLY to them. // If activeSpeakerId is set, listen to primary + active. if (session.primaryUserId) { if (userId === session.primaryUserId) return true; if (session.activeSpeakerId && userId === session.activeSpeakerId) return true; return false; } // Otherwise fall back to allowedUsers list (user IDs) if (this.config.allowedUsers.length === 0) return true; return this.config.allowedUsers.includes(userId); } ``` ### Technical Analysis The authorization decision is based on whether `session.primaryUserId` was successfully resolved, rather than whether the administrator configured `primaryUser`. If the configured primary user cannot be found in the current channel, `resolvePrimaryUser()` leaves `session.primaryUserId` undefined. This can occur when: - The primary user has not joined the channel. - A name-based selector no longer matches the user's current name. - The configured name is ambiguous or misspelled. - The expected member is temporarily unavailable when the bot joins. When `primaryUserId` is und ...[truncated 1856 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Distinguish between an unconfigured primary user and a configured but unresolved primary user: ```ts private isUserAllowed(session: VoiceSession, userId: string): boolean { if (this.config.primaryUser) { if (!session.primaryUserId) { return false; } return userId === session.primaryUserId || userId === session.activeSpeakerId; } if (this.config.allowedUsers.length === 0) { return false; // Prefer deny-by-default } return this.config.allowedUsers.includes(userId); } ``` 2. Do not start audio listeners until a configured primary user has been resolved successfully. 3. Prefer immutable Discord user IDs over usernames or display names. Validate ID-based configuration during startup. 4. If name-based selectors remain supported, require an exact and unique match. Reject partial or ambiguous matches. 5. Make unrestricted listening an explicit configuration option rather than interpreting an empty allowlist as authorization for everyone. 6. Emit a clear operational error when access-control configuration cannot be resolved, without silently degrading to unrestricted capture. 7. Add automated tests covering: - Configured and resolved primary user. - Configured but absent primary user. - Renamed primary user. - Empty and non-empty allowlists. - Ambiguous display-name matches. ]]>