T09 · Insecure Skill Coding Practices
Error
- Location
- index.js:62
- Finding
- Administrator Credentials and Session Cookies May Be Transmitted over Plaintext HTTP<![CDATA[ ## Vulnerability Details **File Location**: `index.js:62-66`, `index.js:113-155`, `index.js:169-176`, `README.md:40-45`, `README.md:58-65`, `SKILL.md:56-62`, `SKILL.md:199-204`, `SKILL.md:240-244` **Vulnerability Type**: Plaintext transmission of sensitive authentication data **Risk Level**: High ### Vulnerable Code The URL validator explicitly accepts both HTTP and HTTPS: ```javascript function validateUrl(urlStr) { try { const parsed = new URL(urlStr); return (parsed.protocol === 'http:' || parsed.protocol === 'https:') && parsed.hostname; } catch { return false; } } ``` The HTTP client selects the unencrypted Node.js HTTP implementation whenever an `http:` URL is configured. It then sends any session cookie through that connection: ```javascript function httpRequest(baseUrl, endpoint, method = 'GET', postData = null, cookie = null) { return new Promise((resolve, reject) => { const fullUrl = new URL(endpoint, baseUrl); const protocol = fullUrl.protocol === 'https:' ? https : http; const options = { hostname: fullUrl.hostname, port: fullUrl.port || (fullUrl.protocol === 'https:' ? 443 : 80), path: fullUrl.pathname + fullUrl.search, method: method, headers: { 'Content-Type': 'application/json', 'Accept': 'application/json', } }; if (cookie) { options.headers['Cookie'] = cookie; } if (postData) { options.headers['Content-Length'] = Buffer.byteLength(postData); } const req = protocol.request(options, (res) => { const cookies = res.headers['set-cookie']; let cookieValue = null; if (cookies) { cookieValue = cookies.map(c => c.split(';')[0]).join('; '); } let data = ''; res.on('data', chunk => data += chunk); res.on('end', () => { resolve({ statusCode: res.statusCode, data: data, cookie: cookieValue }); }); ...[truncated 3901 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Require HTTPS by default** - Change URL validation to accept only `https:` URLs. - Reject plaintext HTTP before any authentication request is made. ```javascript function validateUrl(urlStr) { try { const parsed = new URL(urlStr); return parsed.protocol === 'https:' && Boolean(parsed.hostname); } catch { return false; } } ``` 2. **Provide an explicit unsafe local-network override only if necessary** - If legacy AdGuard Home deployments require HTTP, require a clearly named setting such as `ADGUARD_ALLOW_INSECURE_HTTP=true`. - Keep the override disabled by default. - Print a prominent warning before transmitting credentials. - Document that the override must only be used over a separately secured transport, such as a loopback connection, trusted tunnel, or mutually authenticated VPN. 3. **Correct all documentation** - Replace every `http://` example in `README.md`, `SKILL.md`, and runtime error messages with `https://`. - Explain that environment variables protect secret storage but do not encrypt network traffic. - Do not describe an HTTP deployment as secure merely because credentials come from environment variables or a secrets manager. 4. **Use least-privilege credentials** - Recommend a dedicated account limited to the monitoring operations required by the skill where AdGuard Home supports such authorization. - Avoid using a general administrator account when a restricted account is available. 5. **Preserve certificate verification** - Continue using Node.js HTTPS certificate verification. - Do not introduce a global `rejectUnauthorized: false` workaround for private certificates. - Support a configured private certificate authority when deployments use an internal PKI. 6. **Add regression tests** - Verify that `http://` configurations are rejected by default. - Verify that `https://` configurations remain accepted. ...[truncated 218 chars]