T09 · Insecure Skill Coding Practices
- Location
lib/config.py:80- Finding
OpenClaw Configuration Permissions Are Not Preserved During Atomic Replacement
- Content
View full analysis
Vulnerability Details
File Location:
lib/config.py:80-104
Vulnerability Type:T09: Insecure Skill Coding Practices
Risk Level: HighVulnerable Code
python def _write_config_locked(self, config: Dict) -> str: """Write config with file locking and backup. Args: config: Config dict to write Returns: Backup file path """ backup_path = f"{self.CONFIG_PATH}.bak-{int(time.time())}" shutil.copy(self.CONFIG_PATH, backup_path) lock_fd = open(self.LOCK_PATH, 'w') try: fcntl.flock(lock_fd, fcntl.LOCK_EX | fcntl.LOCK_NB) # Write to temp file then rename (atomic) tmp_path = f"{self.CONFIG_PATH}.tmp" with open(tmp_path, 'w') as f: json.dump(config, f, indent=2, ensure_ascii=False) os.replace(tmp_path, self.CONFIG_PATH) finally: fcntl.flock(lock_fd, fcntl.LOCK_UN) lock_fd.close() return backup_pathTechnical Analysis
The temporary configuration file is created with Python's default
open(..., 'w')behavior. Its permissions are consequently determined by the process umask instead of inheriting the restrictive permissions of the original~/.openclaw/openclaw.json.For example, under a common
022umask, the temporary file may be created with mode0644. The subsequentos.replace()operation replaces the original configuration inode with this newly created file, so a configuration previously protected with mode0600can become readable by other local users.The OpenClaw configuration contains Discord bot tokens and may contain additional account credentials. This write path is reached by direct permission-removal operations and by the fallback configuration-patching path.
The lock also does not address the permission issue because it only coordinates cooperating writers; it does not control the ...[truncated 1477 chars]
- Remediation
View remediation
Remediation Suggestions
- Preserve the original configuration's owner, group, and mode when replacing it.
- Create the temporary file explicitly with mode
0600, rather than relying on the current umask. For example, useos.open()withO_WRONLY | O_CREAT | O_EXCLand mode0o600, then wrap the descriptor withos.fdopen(). - Use a uniquely named temporary file in the same directory, such as one created through
tempfile.mkstemp(dir=config_directory). - Call
os.fchmod(fd, 0o600)defensively before writing sensitive data. - Flush and call
os.fsync()on the temporary file before replacement, then fsync the parent directory afteros.replace(). - Acquire the lock before reading, copying, or modifying the configuration so the entire read-modify-write transaction is protected.
- Verify after replacement that the resulting file has the expected owner and restrictive mode. Abort or correct the mode if it does not.
- Apply equally restrictive permissions to the lock file and all timestamped backup files.