T05 · Unauthorized Access and Privilege Escalation
- Location
src/ledger.js:15- Finding
Database path restriction can be bypassed through symbolic links
- Content
View full analysis
Vulnerability Details
File Location:
src/ledger.js:15-29, 48-56
Vulnerability Type: Symbolic-link path traversal and filesystem boundary bypass
Risk Level: MediumVulnerable Code
js /** * Validate that a path is within the allowed tally directory. * Rejects path traversal attempts. */ function validateDbPath(dbPath) { const resolved = resolve(dbPath) const allowed = resolved === TALLY_DIR || resolved.startsWith(`${TALLY_DIR}${sep}`) || resolved.startsWith('/tmp/') || resolved.startsWith('/private/tmp/') if (!allowed) { throw new Error('Database path must be within ~/.openclaw/tally/ or /tmp/') } if (resolved.includes('..')) { throw new Error('Path traversal is not allowed') } return resolved }js init() { try { mkdirSync(dirname(this.dbPath), { recursive: true }) validateDbPath(this.dbPath) this.db = new Database(this.dbPath) this.db.pragma('journal_mode = WAL') const migrationPath = resolve(__dirname, '..', 'db', 'migrations', '001_initial.sql') const sql = readFileSync(migrationPath, 'utf-8') this.db.exec(sql) return this } catch (err) { throw new Error('Failed to initialize Tally database') } }Technical Analysis
validateDbPath()performs only lexical path normalization withpath.resolve(). It checks whether the resulting string begins with an allowed directory prefix, but it does not resolve symbolic links or verify the canonical filesystem destination.Consequently, a path such as
/tmp/tally-link/data.dbpasses validation even if/tmp/tally-linkis a symbolic link to a directory outside/tmp. Whenbetter-sqlite3opens the path, the operating system follows the symbolic link and creates or modifies the database at the link target.The secondary
resolved.includes('..')check does not mitigate this issue becauseresolve()has a ...[truncated 1485 chars]- Remediation
View remediation
Remediation Suggestions
- Resolve the canonical path of the nearest existing parent directory with
realpathSync()before opening the database. - Verify that the canonical parent remains under a canonical allowed root using
path.relative()rather than a string-prefix check. - Inspect each existing path component with
lstatSync()and reject symbolic links. - Open or create storage using protections against symbolic-link races where the target platform supports them.
- Remove public support for arbitrary
/tmppaths in production. Provide a separate test-only constructor or dependency-injected storage factory instead. - Create a dedicated temporary directory with
mkdtempSync()and restrictive permissions when temporary databases are required. - Add tests covering symlinked parent directories, symlinked database files, similarly prefixed paths, and time-of-check/time-of-use replacement attempts.
A canonical containment check should follow this pattern:
js import { realpathSync, lstatSync } from 'node:fs' import { relative, isAbsolute } from 'node:path' function isContained(root, targetParent) { const canonicalRoot = realpathSync(root) const canonicalParent = realpathSync(targetParent) const rel = relative(canonicalRoot, canonicalParent) return rel === '' || (!rel.startsWith('..') && !isAbsolute(rel)) }The application must additionally reject symbolic-link components and account for race conditions between validation and opening the file.
- Resolve the canonical path of the nearest existing parent directory with