T09 · Insecure Skill Coding Practices
- Location
src/MessageMetadataExtractor.ts:91- Finding
Ineffective Reply-Depth Tracking and Whitelist Bypass Permit Unbounded Bot Response Loops
- Content
View full analysis
Vulnerability Details
File Location:
src/MessageMetadataExtractor.ts:91-107;src/ResponseRuleEngine.ts:108-114;src/ResponseRuleEngine.ts:134-160
Vulnerability Type: Improper enforcement of message-loop protection
Risk Level: HighVulnerable Code
src/MessageMetadataExtractor.ts:91-107:typescript private calculateDepth(message: Message): number { let depth = 0; // 如果是回复消息，追踪回复链 if (message.reference?.messageId) { depth = 1; // 简化实现：通过内容模式估算深度 // 实际生产中应该 fetch 引用的消息并递归计算 const coordinationPatterns = [ /@\w+\s+(你来 | 你处理 | 你负责 | 请处理)/, /交给\s*@\w+/, /派发给\s*@\w+/, /转交\s*@\w+/, ]; // 如果内容包含协调模式，增加深度 if (coordinationPatterns.some(p => p.test(message.content))) { depth += 1; } } return depth; }src/ResponseRuleEngine.ts:108-114:typescript // 白名单机器人允许 if (isAllowedBot) { return { allowed: true, reason: 'Whitelisted bot', suggestion: 'respond', }; }src/ResponseRuleEngine.ts:134-160:typescript // 规则 3: 深度限制 if (metadata.depth >= this.config.maxDepth) { return { allowed: false, reason: `Max depth exceeded (${metadata.depth} >= ${this.config.maxDepth})`, suggestion: 'ignore', }; } // 规则 4: 会话状态检查 if (sessionState?.isProcessing) { if (this.config.queueWhenBusy) { return { allowed: false, reason: 'Session busy, should queue', suggestion: 'queue', }; } else { return { allowed: false, reason: 'Session busy, dropping message', suggestion: 'ignore', }; } }Technical Analysis
The reply depth is not calculated from the actual reply chain. Every message begins with a depth of zero and is assigned a depth of one whenever it references another message. It may reach two only when the new message text m ...[truncated 2478 chars]
- Remediation
View remediation
Remediation Suggestions
-
Track the real cumulative reply depth.
- Fetch the referenced message and traverse its parent references.
- Enforce a strict traversal limit and detect repeated message IDs to prevent cycles.
- Alternatively, maintain trusted server-side depth metadata keyed by message ID and derive each reply's depth as the parent's depth plus one.
- Do not infer security-sensitive depth solely from message text patterns.
-
Apply universal safety checks before whitelist exceptions.
- Enforce
maxDepthand channel processing-state checks before returning an allowed result for any bot. - Treat the whitelist as permission to participate, not permission to bypass loop and concurrency controls.
- Enforce
-
Require an explicit trigger for whitelisted bots.
- Verify that the guarded bot was directly mentioned or that the message is a reply to a message created by the guarded bot.
- Reject unrelated messages from whitelisted bots.
-
Make mention semantics consistent.
- Replace the contradictory bot and human mention flags with a single unambiguous
mentionsGuardedBotproperty, or define separate flags whose names match their behavior. - Ensure the rule engine consumes only states that the production extractor can generate.
- Replace the contradictory bot and human mention flags with a single unambiguous
-
Add integration tests using extractor-produced metadata.
- Test reply chains longer than
maxDepth. - Test whitelisted bots while a session is processing.
- Test whitelisted and non-whitelisted bot mentions.
- Assert that all bot paths stop at the configured maximum depth.
- Test reply chains longer than
-