T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/query.mjs:34
- Finding
- Arbitrary Local File Disclosure Through Unrestricted --sql-file Input<![CDATA[ ## Vulnerability Details **File Location**: `scripts/query.mjs:34-41`, with the network sink in `scripts/client.mjs:76-98` **Vulnerability Type**: Unrestricted local file read followed by transmission to an external service **Risk Level**: Medium ### Vulnerable Code `scripts/query.mjs:34-41`: ```js let querySql = sql; if (sqlFile) { querySql = readFileSync(sqlFile, "utf8").trim(); } if (!datasetId || !querySql) { console.error("Usage: node query.mjs --dataset-id <dataset_id> --sql \"SELECT ...\" | --sql-file <path>"); process.exit(1); } const result = await callTool("query", { dataset_id: datasetId, sql: querySql }); ``` The corresponding outbound request is implemented in `scripts/client.mjs:76-98`: ```js export async function mcpCall(method, params = {}) { const apiKey = requireEnv("POND3R_API_KEY"); const headers = { "Content-Type": "application/json", Accept: "application/json, text/event-stream", Authorization: `Bearer ${apiKey}`, }; if (_sessionId) { headers["Mcp-Session-Id"] = _sessionId; } const body = { jsonrpc: "2.0", id: Date.now(), method, params, }; const res = await fetch(MCP_URL, { method: "POST", headers, body: JSON.stringify(body), }); ``` ### Technical Analysis The `--sql-file` option accepts an arbitrary filesystem path and passes it directly to `readFileSync`. The implementation does not constrain the path to a designated query directory, reject symbolic links, enforce a file extension or size limit, or verify that the resulting content is a read-only SQL statement. After the file is read, its complete contents are assigned to `querySql`, embedded in the `tools/call` request as the `sql` argument, and transmitted over HTTPS to the fixed Pond3r MCP endpoint. Any remote rejection or server-side SQL validation occurs only after the request body—and therefore the selected file contents—has already been transmitted. The documented “SELECT only” restriction ...[truncated 1869 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Restrict query files to a dedicated directory** - Resolve both the allowed directory and requested path with `realpath`. - Confirm that the canonical requested path remains inside the canonical allowed directory. - Reject absolute paths, traversal sequences, and paths escaping through symbolic links. 2. **Validate the file before transmission** - Permit only regular files with an expected `.sql` extension. - Apply a conservative maximum file size. - Reject null bytes, binary content, and multiple SQL statements. - Parse SQL with a suitable parser and allow only a single read-only `SELECT` statement or explicitly supported read-only CTE. 3. **Minimize filesystem privileges** - Run the Skill under a dedicated account or sandbox with access only to the workspace and approved query directory. - Do not mount secret directories, credential stores, or unrelated host paths into the runtime. 4. **Require explicit authorization for file access** - Before reading a query file, display its canonical path and require user confirmation when operating in an agent-controlled environment. - Do not allow untrusted webpage, chat, dataset, or tool output to choose a local path without confirmation. 5. **Prefer safer input mechanisms** - Accept query text through standard input or a narrowly scoped application interface rather than arbitrary filesystem paths. - Keep local validation mandatory even if Pond3r also enforces read-only SQL on the server. An appropriate canonical-path check should follow this pattern: ```js import { realpathSync, statSync, readFileSync } from "fs"; import { resolve, relative, extname } from "path"; const allowedRoot = realpathSync(resolve(process.cwd(), "queries")); const requestedPath = realpathSync(resolve(allowedRoot, sqlFile)); const rel = relative(allowedRoot, requestedPath); if (rel.startsWith("..") || rel === "" || extname(requestedPath) !== ".sql") { throw new ...[truncated 364 chars]