T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/webhook-server.js:35
- Finding
- Public Webhook Accepts Unauthenticated and Unbounded Requests by Default<![CDATA[ ## Vulnerability Details **File Location**: `scripts/webhook-server.js:35-45, 228-250`; related setup guidance at `SKILL.md:27-33` **Vulnerability Type**: Optional webhook authentication and unbounded request-body handling **Risk Level**: Medium ### Complete Code Snippet ```js function verifySignature(payload, signature, secret) { if (!secret || !signature) return !secret; // Skip if no secret configured const expected = crypto .createHmac('sha256', secret) .update(payload) .digest('hex'); return crypto.timingSafeEqual( Buffer.from(signature), Buffer.from(expected) ); } ``` ```js // Webhook endpoint if (req.method === 'POST' && (req.url === '/' || req.url === '/webhook')) { let body = ''; req.on('data', chunk => { body += chunk; }); req.on('end', async () => { try { // Verify signature if secret is configured const secret = loadSecret(SECRET_PATH); const signature = req.headers['x-hub-signature']; if (secret && !verifySignature(body, signature, secret)) { console.error('Invalid webhook signature'); res.writeHead(401); res.end('Invalid signature'); return; } const parsed = JSON.parse(body); console.log(`Webhook received: ${parsed.eventType} for ${parsed.meetingId}`); const result = await processWebhook(parsed); ``` The related setup documentation explicitly makes the secret optional while recommending public exposure: ```md Runs on port 3142. Expose via Cloudflare Tunnel or ngrok, then paste the URL into Fireflies Settings → Developer Settings → Webhook URL. Optional webhook secret: ```bash echo "YOUR_SECRET" > ~/.openclaw/secrets/fireflies-webhook-secret.txt ``` ``` ### Technical Analysis Webhook authentication is enforced only when the local secret file exists. If the file is absent, the condition guarding `verifySignature` is false and every request reaching `/` or `/webhook` is accepted. This insecure default conflicts with ...[truncated 2606 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Fail closed when no secret is configured.** Refuse to start the webhook server unless a sufficiently strong secret is available. 2. **Require a signature on every webhook request.** Reject both missing and invalid signatures. 3. **Implement the exact signature scheme documented by Fireflies**, including the expected header name and encoding. 4. **Validate signature length before `timingSafeEqual`**, because that function throws when buffer lengths differ: ```js if (!secret || typeof signature !== 'string') return false; const expected = crypto .createHmac('sha256', secret) .update(payload) .digest('hex'); const suppliedBuffer = Buffer.from(signature, 'utf8'); const expectedBuffer = Buffer.from(expected, 'utf8'); return suppliedBuffer.length === expectedBuffer.length && crypto.timingSafeEqual(suppliedBuffer, expectedBuffer); ``` 5. **Enforce a small request-size limit**, destroy the request when exceeded, and return HTTP 413: ```js const MAX_BODY_BYTES = 64 * 1024; let size = 0; const chunks = []; req.on('data', chunk => { size += chunk.length; if (size > MAX_BODY_BYTES) { res.writeHead(413); res.end('Payload too large'); req.destroy(); return; } chunks.push(chunk); }); ``` 6. Validate the parsed payload against a strict schema, including allowed event types and the expected `meetingId` format. 7. Add rate limiting, request timeouts, and replay protection where Fireflies provides a delivery identifier or timestamp. 8. Bind explicitly to `127.0.0.1` by default and require an explicit configuration change for non-loopback exposure. 9. Update `SKILL.md` so webhook authentication is mandatory rather than optional. ]]>