T09 · Insecure Skill Coding Practices
Warning
- Location
- cleaner.py:56
- Finding
- Unvalidated Access-Control Expansion During Channel Merge## Vulnerability Details **File Location**: `cleaner.py`, lines 56–66 **Vulnerability Type**: Unvalidated access-control modification **Risk Level**: Medium ```python if "allowlist" in remove_channel: old = keep_channel.get("allowlist", []) new = remove_channel["allowlist"] merged = list(set(old + new)) keep_channel["allowlist"] = merged print(f"✅ 合并白名单: {merged}") if "dmPolicy" in remove_channel: keep_channel["dmPolicy"] = remove_channel["dmPolicy"] print(f"✅ 设置 dmPolicy: {keep_channel['dmPolicy']}") keep_channel["enabled"] = True ``` ### Technical Analysis The cleaner automatically applies security-sensitive values from the channel selected for removal to the retained channel. It unions both allowlists without validating their types, entries, provenance, or intended authorization scope. It also replaces the retained channel's `dmPolicy` with the discarded channel's policy and unconditionally enables the retained channel. Consequently, a stale, incorrectly configured, or attacker-modified redundant channel can broaden access to the active bot configuration. The merge does not compare policy restrictiveness, request operator approval, or present a security-sensitive configuration diff before writing the changes. ### Attack Path 1. A conflicting `feishu` or `openclaw-feishu` entry exists in `~/.openclaw/openclaw.json`. 2. An attacker or prior unsafe configuration places unauthorized identities in the redundant channel's `allowlist`, assigns it a less restrictive `dmPolicy`, or relies on the active channel currently being disabled. 3. An operator invokes the cleaner to resolve the channel conflict. 4. The cleaner copies the redundant channel's allowlist and direct-message policy into the retained channel without validation or confirmation. 5. The cleaner sets `enabled` to `True` and writes the resulting configuration. 6. After the operator restarts the gateway as instructed, the ...[truncated 634 chars]
- Remediation
- ## Remediation Suggestions - Validate that both channel objects are dictionaries and that each allowlist is a list containing only identifiers of the expected type and format. - Validate `dmPolicy` against an explicit allowlist of supported policy values. - Preserve the retained channel's existing `dmPolicy` by default. Never replace it with a potentially less restrictive policy automatically. - Require explicit operator confirmation before adding identities, weakening a policy, or changing a channel from disabled to enabled. - Do not force `enabled = True`; preserve the retained channel's existing state unless the operator explicitly requests activation. - Calculate and display a redacted configuration diff before applying security-sensitive changes. - Prefer intersection or a documented conflict-resolution rule over an unconditional allowlist union when the authorization intent cannot be established. - Write the updated configuration through a permission-restricted temporary file, flush and synchronize it, and then atomically replace the original file to reduce corruption risk.