T09 · Insecure Skill Coding Practices
- Location
cleaner.py:56- Finding
Unvalidated Access-Control Expansion During Channel Merge
- Content
View full analysis
Vulnerability Details
File Location:
cleaner.py, lines 56–66
Vulnerability Type: Unvalidated access-control modification
Risk Level: Mediumpython 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"] = TrueTechnical 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
dmPolicywith 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
- A conflicting
feishuoropenclaw-feishuentry exists in~/.openclaw/openclaw.json. - An attacker or prior unsafe configuration places unauthorized identities in the redundant channel's
allowlist, assigns it a less restrictivedmPolicy, or relies on the active channel currently being disabled. - An operator invokes the cleaner to resolve the channel conflict.
- The cleaner copies the redundant channel's allowlist and direct-message policy into the retained channel without validation or confirmation.
- The cleaner sets
enabledtoTrueand writes the resulting configuration. - After the operator restarts the gateway as instructed, the ...[truncated 634 chars]
- A conflicting
- Remediation
View 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
dmPolicyagainst an explicit allowlist of supported policy values. - Preserve the retained channel's existing
dmPolicyby 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.