T09 · Insecure Skill Coding Practices
Note
- Location
- scripts/ecommerce_ad_copy_generator_free.py:25
- Finding
- User-Controlled Identifier Is Inserted into Payment URL Without Encoding## Vulnerability Details **File Location**: `scripts/ecommerce_ad_copy_generator_free.py`, lines 25-30 **Vulnerability Type**: Improper encoding of a user-controlled URL query parameter **Risk Level**: Low ```python def _payment_url(user_id: str) -> 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 user-controlled `user_id` directly into a URL template or query string. The input validation elsewhere in the application only requires `user_id` to be nonempty; it does not impose a safe identifier grammar, length limit, or URL encoding. Consequently, reserved URL characters such as an ampersand, number sign, equals sign, or question mark can change the structure or interpretation of the generated payment URL. In the fallback path, an attacker can inject additional query parameters. The configured template path has the same underlying weakness because `str.format` performs no contextual URL encoding. This is a URL construction vulnerability rather than server-side request forgery: the audited code returns the generated URL in JSON but does not make a network request itself. ### Attack Path 1. An attacker supplies a crafted `user_id`, for example through the `--user-id` command-line argument or an input JSON payload. 2. The value passes validation because it is nonempty. 3. `run()` passes the identifier to `_payment_url()`. 4. `_payment_url()` interpolates it without percent-encoding. 5. A value such as `victim&redirect=https://example.invalid` produces a URL containing an injected `redirect` query parameter. 6. The modified URL is returned in the `upgrade.payment_url` field. 7. If a cons ...[truncated 785 chars]
- Remediation
- ## Remediation Suggestions 1. Validate `user_id` against an explicit allowlist appropriate for the identifier format, such as ASCII letters, digits, underscores, and hyphens, and enforce a reasonable maximum length. 2. Build query strings with `urllib.parse.urlencode` rather than direct string concatenation. 3. Parse and reconstruct configured base URLs with `urllib.parse.urlsplit`, `parse_qsl`, and `urlunsplit` so existing query parameters are preserved safely. 4. Avoid applying `str.format` directly to a URL template with unencoded user input. Percent-encode the identifier before substitution, or replace URL templates with structured URL construction. 5. Validate configured payment URLs to permit only expected HTTPS origins if deployment configuration is not fully trusted. 6. Add tests covering ampersands, number signs, equals signs, question marks, Unicode input, duplicate parameters, and base URLs that already contain query strings.