T09 · Insecure Skill Coding Practices
Error
- Location
SKILL.md:346- Finding
Shell Command Injection Through Untrusted Diagram Filenames and Paths
- Content
View full analysis
f.endsWith(".puml")).sort(); let ok = 0, fail = 0; for (const file of files) { const figBase = file.replace(/\.puml$/, ""); const firstLine = (await fs.readFile(path.join(srcDir, file), "utf8")).split(/\r?\n/, 1)[0]; const m = firstLine.match(/@startuml\s+(\S+)/); const alias = m ? m[1] : null; try { for (const fmt of ["svg", "png"]) { const cmd = `docker run --rm -v "${srcDir}:/data" -v "${outDir}:/out" ` + `plantuml/plantuml:latest -charset UTF-8 -output /out -t${fmt} /data/${file}`; execSync(cmd, { stdio: ["ignore", "ignore", "pipe"], windowsHide: true }); } const baseName = alias || figBase; if (baseName !== figBase) { try { await fs.copyFile(path.join(outDir, baseName + ".svg"), path.join(outDir, figBase + ".svg")); } catch {} try { await fs.copyFile(path.join(outDir, baseName + ".png"), path.join(outDir, figBase + ".png")); } catch {} } ok++; } catch (e) { fail++; } } ``` ### Technical Analysis The generated rendering script enumerates `.puml` files and inserts each filename directly into a command string passed to `execSync()`. In Node.js, passing a string to `execSync()` causes the command to be interpreted by a system shell. The `file` value is derived from the input project and is neither validated nor encoded as a shell argument. A filename containing shell metacharacters can therefore alter the command ...[truncated 2017 chars]- Remediation
View remediation
", "-charset", "UTF-8", "-output", "/out", `-t${fmt}`, `/data/${file}`, ]; execFileSync("docker", args, { stdio: ["ignore", "ignore", "pipe"], windowsHide: true, shell: false, }); ``` 2. Validate input filenames before execution. Reject path separators, control characters, null bytes, shell metacharacters, and filenames outside a conservative allowlist. 3. Verify that each resolved input remains under `srcDir`: ```js const inputPath = path.resolve(srcDir, file); const relative = path.relative(srcDir, inputPath); if (relative.startsWith("..") || path.isAbsolute(relative)) { throw new Error(`Invalid diagram path: ${file}`); } ``` 4. Do not attempt to solve the issue through manual quoting alone. Correct escaping varies across POSIX shells, PowerShell, and `cmd.exe`. 5. Run conversion in a sandbox with minimal filesystem access, no unnecessary secrets, restricted network access, and narrowly scoped Docker permissions. 6. Apply the correction to both `SKILL.md` and `SKILL.zh-CN.md` so that generated implementations remain consistent. ]]>