T09 · Insecure Skill Coding Practices
Error
- Location
- src/index.js:66
- Finding
- Command Injection in Agent Log Retrieval## Vulnerability Details **File Location**: `src/index.js`, lines 66-76 **Vulnerability Type**: OS command injection through unvalidated CLI arguments **Risk Level**: High ```js async logs(agentId: string, lines: number = 50): Promise<void> { try { const result = await exec({ command: `tail -n ${lines} /home/nvi/.openclaw/sessions/${agentId}/logs.txt`, timeout: 5 }); console.log(`Logs de ${agentId} (últimas ${lines} líneas):`); console.log(result.stdout); } catch (error) { console.log(`Error al obtener logs: ${error}`); } } ``` The affected parameters are populated directly from CLI arguments: ```js case "logs": if (args.length < 2) { console.log("Uso: logs <agent> [líneas]"); } else { const lines = args.length > 2 ? parseInt(args[2]) : 50; manager.logs(args[1], lines); } break; ``` ### Technical Analysis The `agentId` value is inserted directly into a shell command passed to `exec`. No allowlist validation, shell escaping, canonical path verification, or argument separation is performed. Shell metacharacters in a crafted agent identifier can therefore alter the intended command and append additional shell operations. The `lines` argument is passed through `parseInt`, which limits some direct injection opportunities through that parameter, but the result is not checked for finiteness, positivity, or a safe upper bound. The directly interpolated `agentId` remains the primary command-injection vector. ### Attack Path 1. An attacker obtains the ability to invoke the `colmena-manager logs` CLI, either directly or through a service that exposes this command. 2. The attacker supplies an agent identifier containing shell syntax. 3. The CLI forwards the value through `args[1]` to `manager.logs()`. 4. The value is interpolated into `exec.command`. 5. The system shell interprets the injected syntax and executes attac ...[truncated 531 chars]
- Remediation
- ## Remediation Suggestions - Replace shell-based log retrieval with Node.js filesystem APIs and implement bounded tail-reading logic without invoking a shell. - Validate agent identifiers against a strict allowlist such as `^[A-Za-z0-9_-]+$`. - Resolve the requested log path with `path.resolve()` and verify that it remains beneath the canonical sessions directory. - Reject symbolic links or otherwise ensure they cannot redirect access outside the intended directory. - Require the line count to be a finite integer within a conservative range, such as 1 through 10,000. - If an external executable must be used, call it through `execFile` or `spawn` with a separate argument array and `shell: false`. - Return a nonzero exit status when validation fails, and avoid including sensitive command details in error messages.