T09 · Insecure Skill Coding Practices
Warning
- Location
- src/dyson_cli/config.py:10
- Finding
- Dyson MQTT credentials are stored without enforced restrictive permissions## Vulnerability Details **File Location**: `src/dyson_cli/config.py`, lines 10-28 **Vulnerability Type**: Plaintext sensitive-data storage with insufficient permission enforcement **Risk Level**: Medium ### Vulnerable Code ```python CONFIG_DIR = Path.home() / ".dyson" CONFIG_FILE = CONFIG_DIR / "config.json" def ensure_config_dir() -> Path: """Ensure the config directory exists.""" CONFIG_DIR.mkdir(parents=True, exist_ok=True) return CONFIG_DIR def load_config() -> dict: """Load configuration from disk.""" if not CONFIG_FILE.exists(): return {"devices": [], "default_device": None} return json.loads(CONFIG_FILE.read_text()) def save_config(config: dict) -> None: """Save configuration to disk.""" ensure_config_dir() CONFIG_FILE.write_text(json.dumps(config, indent=2)) ``` The sensitive value written through this function originates in `src/dyson_cli/cli.py`, lines 102-108: ```python for device in devices: device_info = { "name": device.name, "serial": device.serial, "credential": device.credential, "product_type": device.product_type, } config["devices"].append(device_info) ``` ### Technical Analysis Device MQTT credentials are deliberately persisted in `~/.dyson/config.json`, but the code does not explicitly assign restrictive permissions to either the configuration directory or the credential file. `Path.mkdir()` and `Path.write_text()` rely on the process umask and any permissions already present on the path. On a system with a permissive umask, inherited access-control entries, or an existing configuration file with unsafe permissions, other local users may be able to read the serial number, local IP address, and MQTT credential. Storing a local device credential is necessary for the declared local-control functionality, but allowing its confidentiality to depend entirely on ambient operatin ...[truncated 1544 chars]
- Remediation
- ## Remediation Suggestions 1. Create `~/.dyson` with mode `0700` and verify its effective permissions after creation. 2. Create the credential file with mode `0600`; do not rely solely on the caller's umask. 3. When the file already exists, inspect its mode and ACLs. Correct unsafe permissions or refuse to load credentials until the user resolves them. 4. Write configuration atomically through a temporary file in the protected directory, set mode `0600`, flush and synchronize it, and then replace the destination with `os.replace()`. 5. Consider storing credentials in an operating-system secret store or keyring, leaving only non-sensitive device metadata in JSON. 6. Avoid printing credential values in logs or exception messages. 7. Document the sensitivity and required permissions of `~/.dyson/config.json`. A hardened implementation should use explicit permission controls, for example: ```python import json import os import tempfile def ensure_config_dir() -> Path: CONFIG_DIR.mkdir(parents=True, exist_ok=True, mode=0o700) os.chmod(CONFIG_DIR, 0o700) return CONFIG_DIR def save_config(config: dict) -> None: ensure_config_dir() fd, temporary_name = tempfile.mkstemp(dir=CONFIG_DIR) try: os.fchmod(fd, 0o600) with os.fdopen(fd, "w", encoding="utf-8") as stream: json.dump(config, stream, indent=2) stream.flush() os.fsync(stream.fileno()) os.replace(temporary_name, CONFIG_FILE) os.chmod(CONFIG_FILE, 0o600) except Exception: try: os.unlink(temporary_name) except FileNotFoundError: pass raise ```