T09 · Insecure Skill Coding Practices
Warning
- Location
- SKILL.md:163
- Finding
- Secret file write can be redirected through a symlinked or replaced parent directory<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md:163-184` **Vulnerability Type**: Parent-directory symlink traversal and time-of-check/time-of-use race (CWE-59, CWE-367) **Risk Level**: Medium ### Vulnerable Code ```js const project = process.env.ABRA_PROJECT ?? ""; // project is used as a filename: one path segment, no dots at the start, no separators if (!/^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$/.test(project)) { console.error("invalid ABRA_PROJECT"); process.exit(2); } const dir = path.join(os.homedir(), ".abracadabra", "agent-env"); const file = path.resolve(dir, `${project}.json`); if (path.dirname(file) !== path.resolve(dir)) { console.error("refusing path outside agent-env"); process.exit(2); } (async () => { const res = await fetch("http://127.0.0.1:7331/secret", { method: "POST", headers: { authorization: `Bearer ${key}`, "content-type": "application/json" }, body: JSON.stringify({ project, keys: allow }), }); if (!res.ok) { console.error(`abra /secret failed: HTTP ${res.status}`); process.exit(1); } const j = await res.json(); if (j.error) { console.error("abra error (see abra serve log)"); process.exit(1); } const out = {}; for (const k of allow) if (typeof j[k] === "string") out[k] = j[k]; // opaque fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); if (fs.lstatSync(dir).uid !== os.userInfo().uid) { console.error("agent-env dir not owned by user"); process.exit(1); } try { if (fs.lstatSync(file).isSymbolicLink()) { console.error("refusing symlink"); process.exit(1); } } catch {} const fd = fs.openSync(file, "wx", 0o600); // exclusive: fails if it already exists fs.fchmodSync(fd, 0o600); fs.writeSync(fd, JSON.stringify(out)); ``` ### Technical Analysis The lexical `path.resolve` and `path.dirname` checks ensure that the supplied project name does not directly contain a path traversal sequence. They do not, however, verify the physical filesystem path after resolving symbolic links. The code checks ...[truncated 2884 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Reject symbolic links for `~/.abracadabra`, `agent-env`, and every other relevant path component. Do not rely only on the ownership of the final directory. - Resolve the directory with `realpath` after creation and verify that its physical parent is the expected, trusted `~/.abracadabra` directory. - Validate that each component is a directory owned by the expected user and is not writable by group or others. - Avoid separate validation and open operations where possible. Use descriptor-relative filesystem operations with protections equivalent to `openat2` using `RESOLVE_BENEATH`, `RESOLVE_NO_SYMLINKS`, and `RESOLVE_NO_MAGICLINKS` on platforms that support them. - At minimum, open the trusted directory first, retain its directory descriptor, and create the file relative to that descriptor with no-follow and exclusive-creation protections. Revalidate the opened file using `fstat`. - Ensure the file descriptor is closed in a `finally` block so errors during serialization or writing do not leave descriptors open. - Continue preferring the documented process-environment injection workflow, which avoids persistent secret files entirely. - If portable race-free directory-relative operations are unavailable in Node.js, move this sensitive write operation into a small audited native helper or abort when any path component is a symbolic link. ]]>