diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 17f338fe..8a666e97 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -128,3 +128,7 @@ **Vulnerability:** The backend CSV export for audit logs neutralized `=`, `+`, `-`, and `@` but failed to neutralize `|` (pipe) characters, allowing potential DDE (Dynamic Data Exchange) injection if exported logs were opened in spreadsheet software. **Learning:** Spreadsheet formula defenses must cover all command-style prefixes including `|` across all CSV export boundaries, both frontend and backend. **Prevention:** Update the sanitization regex in the backend export function to `/^[=+\-@|]/` so that all potentially executable spreadsheet payloads are prefixed with a single quote. +## 2025-02-27 - [Server-Side Request Forgery (SSRF) in Webhooks] +**Vulnerability:** Webhooks endpoints allowed user-provided URLs to target internal resources and loopback addresses. +**Learning:** `fetch` calls will blindly follow user-provided addresses including internal IP ranges and `localhost`. Node's `fetch` does not intrinsically block these requests. +**Prevention:** Validate user-supplied webhook URLs by checking the parsed `url.hostname` against a blocklist of private network CIDR blocks (127.0.0.0/8, 10.0.0.0/8, etc.) and loopback strings, including obscure forms like IPv4-mapped IPv6 (e.g. `[::ffff:127.0.0.1]`). diff --git a/server/app.mjs b/server/app.mjs index c432a84f..1377538b 100644 --- a/server/app.mjs +++ b/server/app.mjs @@ -89,6 +89,35 @@ const metrics = { attachmentStatusRefreshDeferred: 0, }; +function isSafeUrl(urlString) { + try { + const u = new URL(urlString); + if (u.protocol !== 'http:' && u.protocol !== 'https:') return false; + const h = u.hostname.toLowerCase(); + + // Explicitly block local/private domains and IPs + if (h === 'localhost' || h === '0.0.0.0' || h === '[::1]' || h === '[::]') return false; + + // Block IPv4 loopback and private networks + if (h.startsWith('127.') || + h.startsWith('169.254.') || + h.startsWith('10.') || + h.match(/^172\.(1[6-9]|2[0-9]|3[0-1])\./) || + h.startsWith('192.168.')) { + return false; + } + + // Block IPv6 mapped formats + if (h.includes('7f00:1') || h.includes('::ffff:127.')) { + return false; + } + + return true; + } catch { + return false; + } +} + // Outbound webhooks: POST signed JSON to each active hook subscribed to `event`. // Fire-and-forget with a timeout, one retry on failure, and a recorded outcome // per attempt — never blocks or fails the triggering request. @@ -748,6 +777,7 @@ app.post('/api/orgs/:id/webhooks', requireAuth, async (c) => { if (!canManage(orgRole(uid, orgId))) return c.json({ error: 'forbidden' }, 403); const { url, events } = await c.req.json().catch(() => ({})); if (!/^https?:\/\//.test(String(url || ''))) return c.json({ error: 'valid http(s) url required' }, 400); + if (!isSafeUrl(url)) return c.json({ error: 'unsafe webhook url' }, 400); const secret = `whsec_${randomBytes(24).toString('base64url')}`; const evs = Array.isArray(events) ? events.join(',') : (events || '*'); const id = rowid(db.prepare('INSERT INTO webhooks(org_id,url,secret,events) VALUES(?,?,?,?)').run(orgId, url, secret, evs)); diff --git a/tests/api/smoke.mjs b/tests/api/smoke.mjs index e536b908..9daadcf6 100644 --- a/tests/api/smoke.mjs +++ b/tests/api/smoke.mjs @@ -266,7 +266,7 @@ r = await req(`/api/orgs/${orgAId}/export`, { headers: oauth }); assert.equal(r.status, 403, 'non-owner export → 403'); // ---- Webhooks ---- -r = await req(`/api/orgs/${orgAId}/webhooks`, { method: 'POST', headers: auth, body: body({ url: 'http://127.0.0.1:9/hook', events: ['project.update'] }) }); +r = await req(`/api/orgs/${orgAId}/webhooks`, { method: 'POST', headers: auth, body: body({ url: 'https://example.com/hook', events: ['project.update'] }) }); assert.equal(r.status, 200, 'create webhook'); const wh = await r.json(); assert.ok(wh.secret.startsWith('whsec_'), 'webhook secret returned once');