T09 · Insecure Skill Coding Practices
- Location
export_screen.mjs:87- Finding
Command Injection Through the Single-Screen Export Output Path
- Content
View full analysis
{ try { execSync(`open "${targetPath.replace(/"/g, '\\"')}"`); } catch (err) { log(`Warning: failed to open ${targetPath}: ${err?.message || err}`); } }; ``` ### Technical Analysis The `targetPath` value is interpolated into a command string passed to `execSync`, which executes the command through a shell. The only attempted protection escapes double quotation marks. This does not prevent shell expansions that remain active inside double-quoted strings, including command substitution with `$()` and backticks. A caller can control the output path through the `--workdir ` argument. That path is subsequently supplied to `openInFinder` unless the caller explicitly uses `--no-open`. Consequently, a crafted directory name can inject a shell expression even though the path is enclosed in double quotes. For example, a path containing `$(touch /tmp/export-pwned)` would cause the shell to execute `touch /tmp/export-pwned` while evaluating the command string. ### Attack Path 1. An attacker persuades a user or automation process to run the exporter with a malicious `--workdir` value. 2. The value is accepted without restricting shell metacharacters and is resolved into the export path. 3. The export completes without the `--no-open` option. 4. `openInFinder` interpolates the attacker-controlled path into an `execSync` command string. 5. The shell evaluates command substitution embedded in the path. 6. The injected command executes with the same operating-system privileges as the Node.js exporter. Example: ```bash ZEPLIN_TOKEN="..." node export_screen.mjs PROJECT_ID SCREEN_ID \ --workdir '/tmp/export-$(touch /tmp/export-pwned)' ``` ### Impact Assessment Successful exploitation provides arbitrary command ...[truncated 421 chars]- Remediation
View remediation
{ try { execFileSync("open", [targetPath], { shell: false, stdio: "ignore" }); } catch (err) { log(`Warning: failed to open the generated output: ${err?.message || err}`); } }; ``` Additional hardening should include: - Keep `--no-open` as the default behavior for automated or agent-driven execution. - Validate that a supplied work directory resolves beneath an explicitly permitted export root when arbitrary destinations are unnecessary. - Never attempt to secure shell commands solely by escaping selected metacharacters. - Avoid including attacker-controlled paths in error messages when logs may be consumed by other parsers. ]]>
