T09 · Insecure Skill Coding Practices
Warning
- Location
scripts/build_raingage_section.py:29- Finding
SWMM Configuration Injection Through Unsanitized Raingage Identifiers
- Content
View full analysis
str: hhmm = interval_hhmm(interval_min) lines = [ "[RAINGAGES]", ";;Name Format Interval SCF Source", f"{gage_id:<18} {rain_format:<10} {hhmm:<10} {scf:<8g} TIMESERIES {series_name}", ] return "\n".join(lines) + "\n" ``` The values originate directly from command-line parameters: ```python ap.add_argument("--gage-id", default="RG1") ap.add_argument("--series-name", default=None) ``` ### Technical Analysis `gage_id` and `series_name` are inserted verbatim into a generated SWMM configuration fragment. The code does not enforce SWMM token syntax and does not reject carriage returns, line feeds, whitespace, brackets, semicolons, or other configuration control characters. An attacker who can influence these CLI parameters, including through a wrapper exposing them as agent tool parameters, can terminate the intended raingage row and inject additional SWMM directives or sections. Python field-width formatting such as `<18` only pads short strings; it does not truncate, escape, or sanitize malicious input. This is configuration-language injection rather than operating-system command injection. The reviewed code does not execute the resulting text as shell commands, but downstream SWMM tooling may interpret injected content as trusted model configuration. ### Attack Path 1. An attacker obtains control over `--gage-id` or `--series-name`, directly or through an MCP/agent wrapper. 2. The attacker provides a value containing a newline and additional SWMM syntax, such as a new section header and attacker-selected model options. 3. `bu ...[truncated 892 chars]- Remediation
View remediation
str: if not IDENTIFIER_RE.fullmatch(value): raise ValueError(f"{field} contains unsupported characters") return value ``` - Explicitly reject all control characters, including `\r`, `\n`, and `\t`. - Reject brackets, semicolons, and whitespace even if future changes loosen the identifier expression. - Apply validation after loading `series_name` from rainfall JSON, because that JSON may itself contain untrusted values. - Add negative tests for newline injection, section headers, comments, spaces, tabs, and empty identifiers. - If arbitrary display names must be supported, maintain a separate sanitized internal identifier rather than embedding display text into SWMM syntax. ]]>