T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/return_analyzer.py:33
- Finding
- Unescaped CSV Values Injected into Generated Markdown Reports<![CDATA[ ## Vulnerability Details **File Location**: `scripts/return_analyzer.py`, lines 33–100 **Vulnerability Type**: Stored Markdown/HTML content injection **Risk Level**: Medium ### Vulnerable Code The `product` and `reason` fields are read directly from an attacker-controllable CSV file: ```python def read_csv(path: Path) -> List[ReturnRow]: rows: List[ReturnRow] = [] with path.open(encoding="utf-8") as f: reader = csv.DictReader(f) for row in reader: rows.append(ReturnRow( order_id=row.get("order_id", "").strip(), product=row.get("product", "").strip(), reason=row.get("reason", "").strip(), order_date=row.get("order_date", "").strip(), return_date=row.get("return_date", "").strip(), )) return rows ``` Those fields are subsequently inserted into Markdown tables without escaping or normalization: ```python for reason, count in reason_counts.most_common(): share = count / total_returns * 100 if total_returns else 0 lines.append(f"| {reason} | {count:,} | {share:.1f}% |") ``` ```python if flagged: lines.append("| Product | Returns | Return rate | Top reason |") lines.append("|---------|--------:|------------:|------------|") for product, ret_count, rate, top_reason in flagged: lines.append(f"| {product} | {ret_count:,} | {rate:.1f}% | {top_reason} |") ``` ```python for product, ret_count in product_returns.most_common(): top_reason = product_reasons[product].most_common(1)[0][0] if product_reasons[product] else "unknown" lines.append(f"| {product} | {ret_count:,} | {top_reason} |") ``` ### Technical Analysis CSV fields are treated as trusted report content even though the input file can originate from an external or otherwise untrusted source. Markdown table delimiters, line breaks, links, and raw HTML are not escaped before report generation. An attacker can place Markdown or HTML m ...[truncated 1922 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Escape all untrusted values before inserting them into Markdown tables: - Escape pipe characters as `\|`. - Replace carriage returns and line feeds with spaces. - Remove unsafe control characters. - Encode or reject raw HTML where it is not required. 2. Introduce a dedicated sanitization function and apply it to every CSV-derived field: ```python import html import re def escape_markdown_cell(value: str) -> str: value = value.replace("\r", " ").replace("\n", " ") value = re.sub(r"[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]", "", value) value = html.escape(value, quote=True) return value.replace("|", r"\|") ``` 3. Sanitize values before report formatting: ```python safe_reason = escape_markdown_cell(reason) safe_product = escape_markdown_cell(product) safe_top_reason = escape_markdown_cell(top_reason) ``` 4. Configure the downstream Markdown renderer to disable raw HTML and active content. Sanitization should still occur in the generator because renderer configuration may differ between users. 5. Add regression tests covering: - Pipe characters. - Embedded newlines. - Markdown links and images. - Raw HTML tags and event handlers. - Control characters. - Extremely long field values. 6. Consider generating a structured format such as JSON or a properly escaped HTML document through a trusted templating library when reports must process untrusted external data. ]]>