T09 · Insecure Skill Coding Practices
Error
- Location
- office-to-md/utils/pptConverter.js:65
- Finding
- Shell Command Injection Through a Crafted PPTX File Path<![CDATA[ ## Vulnerability Details **File Location**: `office-to-md/utils/pptConverter.js:65-69` and `office-to-md/utils/pptConverter.js:80-83` **Vulnerability Type**: OS command injection through shell-string interpolation **Risk Level**: High ### Vulnerable Code ```javascript // Execute Python script const result = execSync(`python3 "${tempScriptPath}" "${filePath}"`, { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }); ``` The fallback conversion path contains the same vulnerability: ```javascript // Alternative: use unzip to extract XML and parse try { const tempDir = `/tmp/pptx_${Date.now()}`; execSync(`unzip -q "${filePath}" -d "${tempDir}"`, { stdio: 'pipe' }); ``` ### Technical Analysis `filePath` originates from the command-line argument supplied to the converter. It is inserted directly into command strings passed to `child_process.execSync()`. By default, `execSync()` executes the supplied string through a system shell. Enclosing the value in double quotes does not provide adequate shell escaping. A filename containing a double quote can terminate the quoted argument, after which shell metacharacters can introduce an additional command. The file only needs to exist and retain a `.pptx` extension to pass the validation in `openclaw-skill.js`. Both the primary Python conversion command and the `unzip` fallback are affected. Therefore, exploitation does not depend solely on the fallback path. ### Attack Path 1. An attacker creates or uploads an existing file whose name contains a double quote and shell control characters while still ending in `.pptx`. 2. The attacker causes the Agent or another user to invoke the skill with that file path. 3. `openclaw-skill.js` resolves the path, confirms that the file exists, and accepts its `.pptx` extension. 4. `pptConverter.js` embeds the attacker-controlled path into an `execSync()` shell command. 5. The embedded quote terminates the intended argument, and the shell interprets the remaining fi ...[truncated 572 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Do not construct shell commands by interpolating file paths. Invoke executables directly with argument arrays: ```javascript const { execFileSync } = require('child_process'); const result = execFileSync( 'python3', [tempScriptPath, filePath], { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] } ); execFileSync( 'unzip', ['-q', filePath, '-d', tempDir], { stdio: ['ignore', 'pipe', 'pipe'] } ); ``` Additional hardening measures: 1. Replace `execSync()` with `execFileSync()` or `spawnSync()` wherever arguments can contain external input. 2. Never attempt to implement manual shell escaping as the primary defense. 3. Replace the shell-based cleanup command with: ```javascript fs.rmSync(tempDir, { recursive: true, force: true }); ``` 4. Confirm that the input is a regular file using `fs.statSync()` or `fs.lstatSync()`. 5. Consider validating the file signature rather than relying only on its extension. 6. Run document conversion in a restricted, non-privileged sandbox with filesystem and process limits. ]]>