T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/export_charts.js:119
- Finding
- Untrusted JavaScript Evaluation During Static Chart Export<![CDATA[ ## Vulnerability Details **File Location**: `scripts/report_assembler.py:119-143`, `scripts/report_assembler.py:333-345`, `scripts/report_assembler.py:973-1009`; `scripts/export_charts.js:119-126` **Vulnerability Type**: Evaluation of attacker-controlled JavaScript in a Node.js VM context **Risk Level**: High ### Complete Code Snippet `scripts/report_assembler.py:119-143`: ```python def _resolve_chart_path(chart_path: str, charts_dir: Path, spec_dir: Path) -> Path: """Resolve chart path: as given, beneath charts_dir, or beside the specification.""" candidates = [ Path(chart_path), charts_dir / chart_path, spec_dir / chart_path, ] for cand in candidates: if cand.is_file(): return cand return None def _extract_chart_payload(html_text: str, source: Path) -> dict: start = html_text.find(_OPT_START) if start != -1: end = html_text.find(_OPT_END, start) if end == -1: ... option_js = html_text[start + len(_OPT_START):end].strip() while option_js.endswith(';'): option_js = option_js[:-1].rstrip() if not option_js.startswith('{') or not option_js.endswith('}'): ... ``` `scripts/report_assembler.py:333-345`: ```python def _run_node_export(skill_root: Path, manifest: list, out_dir: Path, timeout: int = 60): manifest_path = out_dir / '_manifest.json' manifest_path.parent.mkdir(parents=True, exist_ok=True) manifest_path.write_text(json.dumps(manifest, ensure_ascii=False), encoding='utf-8') env = os.environ.copy() env['NODE_PATH'] = str(skill_root / 'scripts') try: proc = subprocess.run( ['node', str(skill_root / 'scripts' / 'export_charts.js'), str(manifest_path), str(out_dir)], env=env, capture_output=True, text=True, timeout=timeout, ) ``` `scripts/report_assembler.py:973-1009`: ```python charts_dir = Path(ns.charts_dir).expanduser() ...[truncated 4757 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Eliminate JavaScript evaluation for chart data** - Serialize chart options as strict JSON. - Parse them with `JSON.parse` rather than `vm.runInContext`. - Do not permit functions, getters, computed properties, prototypes, or executable expressions in the manifest. 2. **Replace executable formatters with identifiers** - Where ECharts requires formatter functions, store a fixed formatter identifier such as `"axisTooltipV1"`. - Map that identifier to a hard-coded, reviewed function inside `export_charts.js`. - Reject unknown identifiers rather than accepting arbitrary JavaScript. 3. **Constrain chart file resolution** - Canonicalize the selected file with `Path.resolve()`. - Require the canonical path to be beneath the canonical `charts_dir`. - Do not accept arbitrary absolute paths or traversal outside the designated chart directory unless an explicit trusted mode is enabled. 4. **Validate file provenance and structure** - Add a versioned metadata marker to CLI-generated chart files. - Validate the chart schema and allowed ECharts option fields before export. - Consider recording and verifying a digest of each chart artifact when the report specification is created. 5. **Apply defense-in-depth isolation** - If JavaScript execution cannot immediately be removed, provide a short timeout directly to `vm.runInContext`. - Run rendering in a resource-limited child process or container. - Disable network access and expose only required temporary input and output directories. - Clear unnecessary inherited environment variables. - Enforce process memory and CPU limits. 6. **Add adversarial tests** - Test object literals containing getters, immediately invoked functions, infinite loops, prototype manipulation, and known VM escape patterns. - Verify that non-JSON chart options and paths outside `charts_dir` are rejected before Node is invoked. ]]>