T09 · Insecure Skill Coding Practices
Warning
- Location
- main.py:192
- Finding
- Malformed OpenClaw Configuration Can Be Silently Overwritten<![CDATA[ ## Vulnerability Details **File Location**: `main.py:192-207` **Vulnerability Type**: Unsafe error handling and destructive configuration overwrite **Risk Level**: Medium ### Vulnerable Code ```python def load_openclaw_config() -> dict: """Load OpenClaw configuration.""" if not OPENCLAW_CONFIG_PATH.exists(): return {} try: return json.loads(OPENCLAW_CONFIG_PATH.read_text()) except json.JSONDecodeError: return {} def save_openclaw_config(config: dict): """Save OpenClaw configuration.""" OPENCLAW_CONFIG_PATH.parent.mkdir(parents=True, exist_ok=True) OPENCLAW_CONFIG_PATH.write_text(json.dumps(config, indent=2)) ``` ### Technical Analysis When `~/.openclaw/openclaw.json` contains malformed or partially written JSON, `load_openclaw_config()` silently converts the parsing failure into an empty dictionary. Mutating commands then call `ensure_config_structure()` and reconstruct only the configuration fields needed by FreeRide. The resulting minimal configuration is written directly over the original file. The implementation does not: - Distinguish a nonexistent configuration from a malformed configuration. - Abort configuration changes after a parsing failure. - Create a backup of the original file. - Validate the complete resulting OpenClaw configuration. - Use an atomic temporary-file replacement. This behavior conflicts with the documented claim that unrelated gateway, channel, plugin, environment, custom-instruction, and named-agent settings are preserved. ### Attack Path 1. `~/.openclaw/openclaw.json` becomes invalid JSON. This could result from an interrupted write, manual editing error, filesystem issue, or modification by another local process. 2. The user invokes a mutating operation such as: - `freeride auto` - `freeride switch` - `freeride fallbacks` - A watcher operation that rotates the active model 3. `load_openclaw_config()` catches the parsing error and returns `{}`. ...[truncated 998 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Fail closed when an existing configuration cannot be parsed: ```python def load_openclaw_config() -> dict: if not OPENCLAW_CONFIG_PATH.exists(): return {} try: return json.loads(OPENCLAW_CONFIG_PATH.read_text()) except json.JSONDecodeError as exc: raise RuntimeError( f"Refusing to modify malformed configuration: " f"{OPENCLAW_CONFIG_PATH}" ) from exc ``` 2. Do not invoke any save operation after a load or validation failure. 3. Create a backup before every mutation, using restrictive permissions and a predictable retention policy. 4. Write changes atomically: - Create a temporary file in the same directory. - Set permissions to owner-only where appropriate. - Flush and synchronize the file. - Replace the destination with `os.replace()`. 5. Validate the resulting JSON structure before replacing the active file. 6. Preserve the original file permissions rather than relying on process defaults. 7. Report parsing failures clearly and require the user to repair or explicitly restore the configuration before FreeRide proceeds. ]]>