T09 · Insecure Skill Coding Practices
Warning
- Location
scripts/github_issue_reply_assistant.py:13- Finding
Unencoded User Input Enables Payment URL Query Parameter Injection
- Content
View full analysis
str: template = os.getenv("SKILLPAY_PAYMENT_URL_TEMPLATE", "").strip() if template: return template.format(user_id=user_id) base = os.getenv("SKILLPAY_TOPUP_BASE_URL", "https://skillpay.me/pay").strip() sep = "&" if "?" in base else "?" return f"{base}{sep}user_id={user_id}" ``` ### Technical Analysis The `_payment_url()` function inserts the attacker-controllable `user_id` directly into a URL without percent-encoding it. `validate_payload()` only verifies that `user_id` is non-empty and does not restrict metacharacters such as `&`, `=`, `#`, or `?`. For the fallback construction path, a value such as: ```text victim&amount=100&redirect=https://example.invalid ``` produces a URL resembling: ```text https://skillpay.me/pay?user_id=victim&amount=100&redirect=https://example.invalid ``` The injected delimiters create additional query parameters rather than remaining part of the `user_id` value. The configured template path is similarly unsafe because unrestricted user input is passed directly to `str.format()` without encoding. Exploitation depends on a downstream user, browser, payment service, or integration opening or interpreting the generated `upgrade.payment_url`. The script itself does not make an outbound request or perform a charge. ### Attack Path 1. An attacker supplies an otherwise valid payload with a crafted, non-empty `user_id` containing URL query delimiters. 2. `validate_payload()` accepts the value because it only checks whether `user_id` is empty. 3. `run()` passes the validated identifier to `_payment_url()`. 4. `_payment_url()` concatenates or formats the identifier into the payment URL without percent-encoding. 5. The generated URL is returned in `upgrade.payment_url`. 6. If a ...[truncated 826 chars]- Remediation
View remediation
str: base = os.getenv( "SKILLPAY_TOPUP_BASE_URL", "https://skillpay.me/pay", ).strip() parts = urlsplit(base) query = parse_qsl(parts.query, keep_blank_values=True) query.append(("user_id", user_id)) return urlunsplit( (parts.scheme, parts.netloc, parts.path, urlencode(query), parts.fragment) ) ``` 2. Validate `user_id` against the application's documented identifier format, including a reasonable maximum length. For example, if identifiers are limited to letters, digits, underscores, and hyphens: ```python if not re.fullmatch(r"[A-Za-z0-9_-]{1,128}", user_id): raise ValidationError("`user_id` contains unsupported characters") ``` 3. Avoid free-form URL templates where possible. If template support is required, percent-encode the identifier before substitution and document that `{user_id}` represents an encoded query value. 4. Validate configured payment URLs: - Require HTTPS. - Restrict hosts to an explicit allowlist. - Reject embedded credentials and malformed URLs. - Prevent configuration from redirecting users to untrusted origins. 5. Add tests covering identifiers containing `&`, `=`, `?`, `#`, percent sequences, Unicode characters, and unusually long values. Verify that the resulting URL contains exactly one correctly encoded `user_id` parameter. ]]>