T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/m5stack_firmware_query.py:107
- Finding
- Unrestricted API Base URL Enables Server-Side Request Forgery and Insecure HTTP Communication<![CDATA[ ## Vulnerability Details **File Location**: `scripts/m5stack_firmware_query.py:107-133` **Additional Locations**: `scripts/m5stack_firmware_query.py:19-21, 1393-1397`; `SKILL.md:13`; `references/api.md:13` **Vulnerability Type**: Unrestricted outbound request destination and cleartext HTTP support **Risk Level**: Medium ### Vulnerable Code ```python DEFAULT_BASE_URL = os.environ.get( "M5BURNER_API_BASE_URL", "https://burner.m5stack.com" ).rstrip("/") ``` ```python class FirmwareApi: def __init__(self, base_url: str, timeout: int) -> None: base_url = base_url.strip().rstrip("/") if not base_url.startswith(("http://", "https://")): raise ApiError("base URL must start with http:// or https://") self.base_url = base_url self.timeout = timeout def get( self, path: str, *, params: Mapping[str, Any] | None = None, headers: Mapping[str, str] | None = None, ) -> Any: query = urlencode(compact_params(params or {})) url = f"{self.base_url}/{path.lstrip('/')}" if query: url = f"{url}?{query}" request_headers = { "Accept": "application/json", "User-Agent": USER_AGENT, } request_headers.update(headers or {}) request = Request(url, headers=request_headers, method="GET") try: with urlopen(request, timeout=self.timeout) as response: raw = response.read() status = response.status ``` ```python parser.add_argument( "--base-url", default=DEFAULT_BASE_URL, help="Compatible M5Burner base URL (or M5BURNER_API_BASE_URL)", ) ``` ### Technical Analysis The API client validates only that the supplied base URL begins with `http://` or `https://`. It does not restrict the destination hostname, port, resolved IP address, or transport security. Consequently, either the `--base-url` argument or the `M5BURNER_API_BASE_URL` ...[truncated 3093 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Enforce the production origin by default** - Permit only `https://burner.m5stack.com` during normal operation. - Compare parsed, normalized scheme, hostname, and port rather than using a string prefix check. 2. **Require HTTPS** - Reject `http://` endpoints outside an explicitly enabled local-test mode. - Do not permit transport downgrades from HTTPS to HTTP. 3. **Separate test configuration from production configuration** - Replace unrestricted `--base-url` behavior with an explicit option such as `--allow-test-endpoint`. - Require deliberate operator approval before connecting to a non-production origin. - Avoid allowing an ambient environment variable to silently redirect production requests. 4. **Block sensitive network destinations** - Resolve the destination hostname and reject loopback, private, link-local, multicast, unspecified, and reserved addresses. - Apply the check to every resolved address to reduce DNS rebinding risk. - Re-resolve and revalidate as close as possible to connection establishment. 5. **Control redirects** - Disable automatic redirects where they are unnecessary. - Otherwise, validate the scheme, hostname, port, and resolved address of every redirect target. - Reject cross-origin redirects and HTTPS-to-HTTP redirects. 6. **Treat API content as untrusted** - Clearly delimit firmware descriptions, comments, developer text, and source URLs as external data. - Ensure downstream agents do not interpret returned catalog text as instructions. - Validate response size and expected schema before processing. 7. **Add security tests** - Confirm rejection of loopback, private, link-local, and reserved destinations. - Confirm rejection of cleartext HTTP in production mode. - Confirm cross-origin and downgrade redirects are rejected. - Confirm the official HTTPS endpoint remains functional. ]]>