T09 · Insecure Skill Coding Practices
- Location
- pharmgx_reporter.py:751
- Finding
- Unescaped Input Permits Markdown Report Injection## Vulnerability Details **File Location**: `pharmgx_reporter.py:751-777, 930-936, 998, 1044` **Vulnerability Type**: Markdown content injection through untrusted genotype values and input filenames **Risk Level**: Medium ### Vulnerable Code ```python def parse_file(path): content = Path(path).read_text() lines = content.split("\n") fmt = detect_format(lines) snps = {} for line in lines: if line.startswith("#") or line.strip() == "": continue if "rsid" in line.lower() and "chromosome" in line.lower(): continue parts = line.split("\t") if "\t" in line else line.split(",") if len(parts) >= 4: rsid = parts[0].strip() if not rsid.startswith("rs"): continue if len(parts) == 5: genotype = parts[3].strip() + parts[4].strip() else: genotype = parts[3].strip() if genotype and genotype not in ("--", "00"): snps[rsid] = genotype.upper() ``` ```python checksum = hashlib.sha256(Path(input_path).read_bytes()).hexdigest() fname = Path(input_path).name lines = [] lines.append("# ClawBio PharmGx Report") lines.append("") lines.append(f"**Date**: {now}") lines.append(f"**Input**: `{fname}`") ``` ```python for rsid, info in sorted(pgx_snps.items(), key=lambda x: x[1]["gene"]): lines.append(f"| {rsid} | {info['gene']} | {info['allele']} | {info['genotype']} | {info['effect']} |") ``` ```python lines.append("```bash") lines.append(f"python pharmgx_reporter.py --input {fname} --output report") lines.append("```") ``` ### Technical Analysis The parser accepts genotype fields without enforcing an expected genotype grammar. It only uppercases the supplied text and excludes the exact values `--` and `00`. Report generation then interpolates those values directly into a Markdown table without escaping Markdown control characters. The input basename is also attacker-controlled ...[truncated 2448 chars]
- Remediation
- ## Remediation Suggestions 1. **Strictly validate genotype fields before storing them.** Define a narrow grammar for every supported input format and reject malformed records. For ordinary SNP genotypes, permit only expected nucleotide symbols and lengths. Handle supported insertion, deletion, or repeat encodings through explicit per-variant rules rather than accepting arbitrary text. ```python VALID_SNP_GENOTYPE = re.compile(r"^[ACGT]{1,2}$") if not VALID_SNP_GENOTYPE.fullmatch(genotype.upper()): raise ValueError(f"Invalid genotype for {rsid}") ``` 2. **Escape all values inserted into Markdown tables.** At minimum, neutralize pipes, backslashes, carriage returns, and line feeds. ```python def markdown_table_cell(value): return ( str(value) .replace("\\", "\\\\") .replace("|", "\\|") .replace("\r", " ") .replace("\n", " ") ) ``` 3. **Sanitize the displayed filename separately for each Markdown context.** Inline code spans, table cells, and fenced code blocks have different escaping requirements. Reject or safely encode embedded backticks and line breaks. 4. **Do not construct the reproducibility command through raw interpolation.** Use `shlex.quote()` to render the filename as a shell argument, while also preventing fence termination: ```python import shlex safe_command = ( f"python pharmgx_reporter.py --input " f"{shlex.quote(fname)} --output report" ) safe_command = safe_command.replace("```", "` ` `") ``` 5. **Consider rendering untrusted values as encoded plain text.** A Markdown library with context-aware escaping is preferable to manual interpolation. 6. **Add negative security tests.** Test genotypes and filenames containing `|`, backticks, newlines, brackets, Markdown images, HTML tags, and fenced-code delimiters. Verify that generated reports preserve their intended structure and do not create active links or remote res ...[truncated 186 chars]
