T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/preflight.py:43
- Finding
- Required dependency declaration validation can be bypassed<![CDATA[ ## Vulnerability Details **File Location**: `scripts/preflight.py:43-56` **Vulnerability Type**: Improper validation of hierarchical YAML configuration **Risk Level**: Medium ### Vulnerable Code ```python # inline form: bins: [python3, jq] (also bins: []) m = re.search(r"bins:\s*\[(.*?)\]", block, re.DOTALL) if m: inner = m.group(1) inner = re.sub(r"#.*", "", inner) # strip trailing comments bins = [b.strip().strip("'\"") for b in inner.split(",")] return True, [b for b in bins if b] # block form: # bins: # - python3 m = re.search(r"bins:\s*\n((?:\s*-\s*.+\n?)+)", block) if m: bins = re.findall(r"-\s*(.+?)\s*$", m.group(1), re.MULTILINE) return True, [b.strip().strip("'\"") for b in bins if b.strip()] return False, [] ``` ### Technical Analysis The checker is intended to verify that `metadata.openclaw.requires.bins` is declared. However, `declared_bins()` searches the entire raw frontmatter block for any text matching `bins:`. It does not validate the YAML hierarchy, indentation, parent mappings, or whether the match appears inside a comment. Consequently, unrelated or commented content such as the following may be treated as a valid dependency declaration: ```yaml --- name: example description: Example skill version: 1.0.0 # bins: [] --- ``` An unrelated property can produce the same result: ```yaml --- name: example description: Example skill version: 1.0.0 unrelated: bins: [] --- ``` In both cases, the inline regular expression can set `declared=True` and return an empty binary list. No `bin:<name>` checks are then created. If all other checks pass, the final `all()` validation gate can emit `ok: true`, even though `metadata.openclaw.requires.bins` is absent. This is a fail-open validation flaw and contradicts the documented outcome contract that `ok:true` means every specified manifest check passed. ### Attack Path 1. An attacker or Skill ...[truncated 1298 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Parse the frontmatter with a safe YAML parser rather than searching raw YAML with unrestricted regular expressions. 2. Explicitly verify that: - `metadata` is a mapping. - `metadata.openclaw` is a mapping. - `metadata.openclaw.requires` is a mapping. - `metadata.openclaw.requires.bins` exists and is a list. - Every list element is a non-empty string with an expected binary-name format. 3. Reject malformed YAML and incorrect data types instead of treating them as absent or valid. 4. If the standard-library-only requirement must be retained, implement an indentation-aware parser for this exact hierarchy that ignores comments and quoted occurrences. Do not use an unanchored search across the complete frontmatter block. 5. Add regression tests covering: - `# bins: []` in a comment. - Top-level `bins: []`. - `bins: []` under an unrelated mapping. - Missing `metadata`, `openclaw`, or `requires` mappings. - Scalar, mapping, and null values in place of a list. - Correct inline and block-list declarations at the required hierarchy. 6. Ensure the final validation gate fails closed whenever the frontmatter cannot be parsed unambiguously. ]]>