T09 · Insecure Skill Coding Practices
- Location
scripts/contract_renewal_agent.py:11- Finding
Contract Database Is Created Without Restrictive File Permissions
- Content
View full analysis
Vulnerability Details
File Location:
scripts/contract_renewal_agent.py, lines 11–21
Vulnerability Type: Insecure local storage permissions and unsafe file replacement
Risk Level: MediumVulnerable Code
python DB_PATH = Path.home() / ".openclaw" / "workspace" / "contract-renewal-agent" / "contracts.json" def load_db() -> dict: if not DB_PATH.exists(): DB_PATH.parent.mkdir(parents=True, exist_ok=True) return {"contracts": {}} with open(DB_PATH) as f: return json.load(f) def save_db(db: dict) -> None: with open(DB_PATH, "w") as f: json.dump(db, f, indent=2, default=str)Technical Analysis
The application stores contract names, counterparties, annual values, dates, and notes in a plaintext JSON database. It creates the parent directory and database without explicitly enforcing restrictive permissions such as
0700for the directory and0600for the file. Effective permissions therefore depend on the process umask, and existing insecure permissions are not corrected.The database is also written directly with
open(DB_PATH, "w"). This operation follows a pre-existing symbolic link and truncates the target before writing. If another local user can modify the database directory or replace the database path, the save operation may overwrite another file writable by the victim account. The direct write is also non-atomic, so interruption or concurrent writes can leave the database partially written or corrupted.Exploitation requires local access and sufficient filesystem permissions to read the resulting database or manipulate its path or parent directory.
Attack Path
- The victim runs an operation that invokes
save_db, such as adding, updating, or deleting a contract. - The program creates or writes
contracts.jsonusing permissions derived solely from the environment's umask. - Under a permissive umask or pre-existing unsafe permissions, another local account reads t ...[truncated 933 chars]
- The victim runs an operation that invokes
- Remediation
View remediation
Remediation Suggestions
- Create the database directory with mode
0700and verify that it is owned by the current user. - Create database files with mode
0600, independently of the process umask. - Correct unsafe permissions on an existing database after verifying its ownership.
- Reject symbolic links for the database path and verify each relevant path component before writing.
- Serialize into a securely created same-directory temporary file using exclusive creation and mode
0600. - Flush buffered data, call
fsync, and atomically replace the database withos.replace. - Consider applying an advisory lock to prevent concurrent writers from losing updates or corrupting the JSON document.
- For highly sensitive contract data, add encryption at rest with keys stored separately in an operating-system credential facility.
- Handle malformed JSON and I/O failures without replacing the last valid database.
A hardened implementation should follow this pattern:
python import json import os import tempfile def save_db(db: dict) -> None: DB_PATH.parent.mkdir(parents=True, exist_ok=True, mode=0o700) os.chmod(DB_PATH.parent, 0o700) if DB_PATH.is_symlink(): raise RuntimeError("Refusing to write through a symbolic link") fd, temp_name = tempfile.mkstemp( prefix=".contracts-", suffix=".tmp", dir=DB_PATH.parent, ) try: os.fchmod(fd, 0o600) with os.fdopen(fd, "w") as f: json.dump(db, f, indent=2, default=str) f.flush() os.fsync(f.fileno()) os.replace(temp_name, DB_PATH) os.chmod(DB_PATH, 0o600) except Exception: try: os.unlink(temp_name) except FileNotFoundError: pass raise- Create the database directory with mode