T09 · Insecure Skill Coding Practices
Error
- Location
- ssh-manager.en.html:360
- Finding
- Shell Command Injection Through Web UI Configuration Serialization<![CDATA[ ## Vulnerability Details **File Location**: `ssh-manager.en.html:360-370` **Equivalent Location**: `ssh-manager.html:360-370` **Vulnerability Type**: Shell command injection **Risk Level**: Critical ### Vulnerable Code ```javascript // Read existing config const configResponse = await window.openclawExec?.('cat ~/.openclaw/credentials/ssh-batch.json'); let config = JSON.parse(configResponse); // Add new server config.servers.push(serverData); // Save config using Python json module (SECURE) const tempFile = `/tmp/ssh_config_${Date.now()}.json`; const jsonContent = JSON.stringify(config, null, 2).replace(/'/g, "'\\''"); await window.openclawExec?.(`python3 -c "import json; f=open('${tempFile}','w'); f.write('${jsonContent}'); f.close()"`); await window.openclawExec?.(`mv ${tempFile} ~/.openclaw/credentials/ssh-batch.json`); await window.openclawExec?.('chmod 600 ~/.openclaw/credentials/ssh-batch.json'); ``` ### Technical Analysis The Web UI serializes configuration containing user-controlled `user`, `host`, and related server values, then interpolates the resulting JSON into a shell command passed to `window.openclawExec`. The attempted escaping only transforms single quotation marks. It does not neutralize shell metacharacters or command substitutions such as `$()` and backticks. Moreover, the command is enclosed in shell double quotes, while serialized JSON itself contains double quotes. The JSON can therefore alter the intended shell quoting context. Using Python inside the generated command does not provide protection because the shell parses and expands the complete command before Python is started. The same unsafe construction is used in the server deletion path at `ssh-manager.en.html:388-397` and its counterpart in `ssh-manager.html`. ### Attack Path 1. An attacker supplies a crafted username or hostname through the server form or causes a crafted value to exist in the configuration. 2. The value is included in `serverData` and th ...[truncated 916 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Eliminate dynamically generated shell commands for configuration updates. 2. Pass configuration data to a dedicated helper through stdin or a structured API. 3. Change `openclawExec` to accept an executable and argument array without invoking a shell. 4. Reuse a hardened helper such as `add-server.py`, but add strict schema validation before writing data. 5. Validate usernames, hostnames, and ports server-side: - Restrict usernames to an explicitly permitted character set. - Accept only validated DNS names or IP addresses for hosts. - Require an integer port in the range 1–65535. 6. Write configuration atomically using Python file APIs and a mode-0600 temporary file created with `tempfile`. 7. Apply the same correction to the deletion path and both language variants of the HTML file. 8. Add regression tests using values containing `$()`, backticks, quotes, newlines, semicolons, and redirection operators. ]]>
