T09 · Insecure Skill Coding Practices
Error
- Location
- index.js:4
- Finding
- Arbitrary Local File Read Through Path Traversal## Vulnerability Details **File Location**: `index.js`, lines 4-11 **Vulnerability Type**: Path traversal leading to unauthorized local file disclosure **Risk Level**: High ### Vulnerable Code ```js const filename = params.filename || "example.txt"; const path = `C:\\Users\\user\\.openclaw\\workspace\\${filename}`; if (!fs.existsSync(path)) { return `File ${filename} does not exist in workspace.`; } const content = fs.readFileSync(path, "utf-8"); const summary = content.slice(0, 500) + (content.length > 500 ? "..." : ""); ``` ### Technical Analysis The attacker-controlled `params.filename` value is directly concatenated with the intended workspace directory. The implementation does not reject absolute paths, normalize the resulting path, resolve canonical paths, or verify that the resolved target remains inside the workspace. On Windows, a filename containing parent-directory components such as `..\` can cause the resulting path to resolve outside `C:\Users\user\.openclaw\workspace`. The `fs.existsSync()` check only determines whether the constructed path exists; it does not provide any security boundary. If the path exists, `fs.readFileSync()` reads it with the privileges of the running process. This behavior contradicts the statement in `SKILL.md` that file paths are validated against directory traversal. ### Attack Path 1. An attacker causes the skill to run with a crafted `filename` parameter, such as `..\..\..\sensitive.txt`. 2. The skill appends that value to the configured workspace path. 3. Windows resolves the embedded parent-directory components, moving the effective target outside the workspace. 4. `fs.existsSync()` confirms that the external target exists. 5. `fs.readFileSync()` reads the target as UTF-8. 6. The skill returns the first 500 characters of the file, disclosing its contents to the requester. Exploitation requires knowledge or discovery of a readable target path. The target mu ...[truncated 682 chars]
- Remediation
- ## Remediation Suggestions 1. Define and canonicalize the permitted workspace root using Node.js `path.resolve()`. 2. Require `params.filename` to be a non-empty string and reject null bytes and absolute paths. 3. Resolve the requested path relative to the workspace root. 4. Use `path.relative()` to verify that the resolved target remains within the workspace. Reject paths whose relative form is `..`, begins with `..` plus a path separator, or is absolute. 5. Where symlinks or junctions may exist, compare `fs.realpath()` results for the workspace and target to prevent traversal through filesystem links. 6. Prefer the platform's declared `workspace.read` capability instead of direct unrestricted filesystem access. 7. Avoid exposing attacker-controlled filenames in error messages unless safely encoded for the output context. 8. Add tests for Windows and POSIX traversal forms, absolute paths, mixed separators, drive-qualified paths, UNC paths, symlinks, and junctions. Example containment approach: ```js import fs from "fs"; import path from "path"; const workspaceRoot = path.resolve("C:\\Users\\user\\.openclaw\\workspace"); export async function run(params) { if (typeof params?.filename !== "string" || !params.filename) { throw new Error("A valid filename is required."); } if (path.isAbsolute(params.filename) || params.filename.includes("\0")) { throw new Error("Invalid filename."); } const requestedPath = path.resolve(workspaceRoot, params.filename); const relativePath = path.relative(workspaceRoot, requestedPath); if ( relativePath === ".." || relativePath.startsWith(`..${path.sep}`) || path.isAbsolute(relativePath) ) { throw new Error("Requested file is outside the workspace."); } const realRoot = fs.realpathSync(workspaceRoot); const realTarget = fs.realpathSync(requestedPath); const realRelative = path.relative(realRoot, realTarget); if ( ...[truncated 336 chars]