T09 · Insecure Skill Coding Practices
- Location
src/db/connection.py:27- Finding
Ledger Name Path Traversal Enables Filesystem Writes Outside the Storage Root
- Content
View full analysis
Vulnerability Details
File Location:
src/db/connection.py:27-38
Vulnerability Type: Path traversal and unrestricted filesystem write
Risk Level: Medium
Exposed Inputs:src/cli.py:475-476,src/cli.py:487-488,src/cli.py:495-496,src/cli.py:503-504Vulnerable Code
python def get_db_path(ledger_name: str, base_path: Optional[str] = None) -> str: """ Get the database file path for a ledger. Args: ledger_name: Name of the ledger base_path: Base path for ledger data (default: ~/.openclaw/skills_data/ledger/) Returns: Full path to the SQLite database file """ if base_path is None: base_path = os.path.expanduser("~/.openclaw/skills_data/ledger") # If ledger name is ASCII (English), convert to lowercase if is_ascii(ledger_name): ledger_name = ledger_name.lower() ledger_dir = os.path.join(base_path, ledger_name) os.makedirs(ledger_dir, exist_ok=True) return os.path.join(ledger_dir, "ledger.db")The affected value is accepted directly from CLI arguments, including:
python create_parser.add_argument('--name', type=str, required=True, help='账本名称') show_parser.add_argument('--name', type=str, default=None, help='账本名称') chart_parser.add_argument('--name', type=str, nargs='+', default=None, help='账本名称（支持多个）') add_parser.add_argument('--name', type=str, default=None, help='账本名称')Technical Analysis
ledger_nameis incorporated into a filesystem path without rejecting absolute paths, parent-directory components, path separators, or symlink-based escapes.If
ledger_nameis absolute,os.path.join(base_path, ledger_name)discardsbase_path. A relative value containing..can likewise escape the intended~/.openclaw/skills_data/ledgerdirectory. The resulting path is passed toos.makedirs()and subsequently to SQLite initialization or connection functions.Consequently, commands that create or write led ...[truncated 1982 chars]
- Remediation
View remediation
Remediation Suggestions
-
Apply a conservative allowlist to ledger names. Permit only expected letters, digits, spaces, underscores, and hyphens, with a reasonable maximum length.
-
Explicitly reject:
- Absolute paths.
/and\path separators..and..path components.- NUL characters and control characters.
- Empty or whitespace-only names.
-
Canonicalize the root and candidate paths and enforce containment before creating any directory:
python import re from pathlib import Path LEDGER_NAME_RE = re.compile(r"^[\w \-]{1,100}$", re.UNICODE) def get_db_path(ledger_name: str, base_path: Optional[str] = None) -> str: if not isinstance(ledger_name, str) or not LEDGER_NAME_RE.fullmatch(ledger_name): raise ValueError("Invalid ledger name") if Path(ledger_name).is_absolute() or ledger_name in {".", ".."}: raise ValueError("Invalid ledger name") root = Path( base_path or "~/.openclaw/skills_data/ledger" ).expanduser().resolve() normalized_name = ledger_name if not is_ascii(ledger_name) else ledger_name.lower() ledger_dir = (root / normalized_name).resolve() try: ledger_dir.relative_to(root) except ValueError: raise ValueError("Ledger path escapes the storage root") ledger_dir.mkdir(parents=True, exist_ok=True) return str(ledger_dir / "ledger.db") -
Account for symlink attacks. If the storage directory may be writable by untrusted users, reject symlink components or use filesystem operations designed to avoid following symlinks.
-
Apply the same path-validation routine to migration helpers such as
get_ledger_path()so all ledger-related paths share one trusted implementation. -
Add regression tests covering:
- Absolute Unix and Windows paths.
../and nested traversal sequences.- Mixed path separators.
- Symlink escapes.
- Empty, oversized, and control-chara ...[truncated 56 chars]
-