T09 · Insecure Skill Coding Practices
Error
- Location
- workflows/modules/export_module.js:201
- Finding
- Shell Command Injection Through an Untrusted PRD Title<![CDATA[ ## Vulnerability Details **File Location**: `workflows/modules/export_module.js:201-225`, with the untrusted title extracted at `workflows/modules/export_module.js:352-358` **Vulnerability Type**: Shell command injection **Risk Level**: High ### Vulnerable Code ```javascript const cmd = `"${cliWrapper}" create ` + `--type report ` + `--output "${absoluteOutputPath}" ` + `--title "${this.extractTitle(prd)}" ` + `--toc ` + `--page-numbers ` + `--content-json "${contentJsonPath}"`; execSync(cmd, { encoding: 'utf8', cwd: outputDir, timeout: 60000 }); ``` The fallback execution branch has the same issue: ```javascript const cmd = `"${dotnetCmd}" run --project "${cliProject}" -- create ` + `--type report ` + `--output "${absoluteOutputPath}" ` + `--title "${this.extractTitle(prd)}" ` + `--toc ` + `--page-numbers ` + `--content-json "${contentJsonPath}"`; execSync(cmd, { encoding: 'utf8', cwd: outputDir, timeout: 60000 }); ``` The title is extracted directly from PRD content: ```javascript extractTitle(prd) { const content = prd.content || ''; const match = content.match(/^#\s+(.+)$/m); if (match) { return match[1].trim(); } return '产品需求文档'; } ``` ### Technical Analysis `extractTitle()` returns an unvalidated Markdown heading supplied by the PRD. That value is inserted inside a command string passed to Node.js `execSync()`. String-based `execSync()` invokes a shell, so double quotes alone do not provide safe argument isolation. A malicious heading can contain a closing quote, command substitution, or other shell metacharacters. When Word export uses the `yh-minimax-docx` path, the shell interprets the injected syntax instead of treating the entire title as data. Both the wrapper-script branch and the `dotnet run` fallback branch are affected. ### Attack Path 1. An attacker supplies, imports, or causes the workflow to generate a PRD containing a crafted first-level Markdown heading. 2. The PRD is ...[truncated 841 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Replace string-based `execSync()` with `execFileSync()` or `spawnSync()` and supply arguments as an array: ```javascript execFileSync(cliWrapper, [ 'create', '--type', 'report', '--output', absoluteOutputPath, '--title', this.extractTitle(prd), '--toc', '--page-numbers', '--content-json', contentJsonPath ], { encoding: 'utf8', cwd: outputDir, timeout: 60000 }); ``` 2. Apply the same argument-array approach to the `dotnet` fallback. 3. Do not rely on shell escaping as the primary defense. 4. Validate titles for reasonable length and reject control characters. 5. Add regression tests using titles containing quotes, semicolons, command substitutions, newlines, and platform-specific shell metacharacters. 6. Run document conversion under a restricted account or sandbox with minimal filesystem and network permissions. ]]>