Skip to content

Webhook test endpoint can be abused as a blind SSRF timing/status oracle to probe internal network services #96

Description

@prodbycorne

Overview

POST /api/v1/webhooks/:id/test (src/routes/webhooks.js) lets the owner of a registered webhook trigger an on-demand delivery attempt and returns delivery metadata directly in the response:

router.post('/webhooks/:id/test', testLimit, async (req, res, next) => {
  const delivery = await dispatcher.sendTest(req.params.id);
  if (!delivery) return next(new AppError('NOT_FOUND', 'Webhook not found', 404));
  return res.status(202).json({
    delivery_id: delivery.id,
    status: delivery.status,
    attempts: delivery.attempts,
    response_status: delivery.response_status,
    last_error: delivery.last_error,
  });
});

Combined with the companion SSRF issue (webhook URL registration accepts private/internal network targets with no validation), this endpoint becomes a direct, rate-limited-but-repeatable oracle: an attacker registers a webhook pointing at an internal address/port (http://10.0.0.5:5432/, http://redis:6379/, http://169.254.169.254/latest/meta-data/iam/security-credentials/, or any other internal Docker/VPC-network host), then calls /webhooks/:id/test repeatedly. The response directly leaks:

  • response_status — distinguishes "port closed/connection refused" (postOnce's axios.post rejects with a network error, surfaced via last_error) from "port open, got an HTTP response" (even a non-2xx one, since validateStatus: () => true in postOnce accepts any status) from "port open, service responded with 2xx" (status: 'success') — enough to fingerprint which internal hosts/ports exist and are reachable.
  • last_error — the raw underlying network error message (e.g. ECONNREFUSED, ETIMEDOUT, ECONNRESET), which differs meaningfully by failure mode and further aids network reconnaissance (ECONNREFUSED = host up, port closed; ETIMEDOUT = host firewalled/unreachable; a real HTTP response = port open and speaking HTTP).
  • Timing: even without the response body being returned to the attacker, dispatcher.attempt()'s use of axios with a config.webhooks.timeoutMs bound means a distinguishable time-to-respond is implicitly observable via how quickly the attacker's own subsequent poll of GET /webhooks/:id/deliveries reflects a resolved status.

WEBHOOK_TEST_RATELIMIT_MAX (default 5/min) slows this down but does not prevent it — 5 requests/minute is still enough to slowly but reliably fingerprint an internal network's topology over time, and nothing distinguishes or specifically restricts targets of the test endpoint beyond the general per-IP rate limit that applies to all webhook management actions equally.

Requirements

  • This issue should be resolved primarily by the companion SSRF-prevention work (rejecting private/internal targets at registration and delivery time) — but track it separately because the test endpoint specifically deserves additional hardening given it is a purpose-built, on-demand, attacker-triggerable oracle in a way that a random passive pool.* event dispatch is not.
  • Once basic SSRF protection lands, verify explicitly that /webhooks/:id/test is also covered (it shares dispatcher.deliverToWebhook under the hood via sendTest, so it should be, but add a dedicated test proving the test endpoint specifically cannot be pointed at an internal target even if some future refactor changes its code path).
  • Consider reducing the information returned by the test endpoint response — e.g. do not return the raw last_error network error string to the caller (log it server-side, but return a generic "delivery failed" externally) since the granular error message itself is part of what makes this a useful reconnaissance oracle even after SSRF filtering closes the direct-request vector (defense in depth against any SSRF-filter bypass).
  • Ensure the per-IP WEBHOOK_TEST_RATELIMIT_MAX limiter cannot be trivially bypassed by registering many different webhooks and testing each once (i.e. consider whether rate limiting should also account for per-webhook or per-account test frequency, not just per-IP).

Acceptance Criteria

  • /webhooks/:id/test is verified (via a dedicated test, independent of the general webhook-delivery SSRF test) to refuse testing a webhook whose URL targets a private/internal/link-local address.
  • The externally-visible last_error field on the test endpoint's response no longer leaks raw low-level network error codes (ECONNREFUSED/ETIMEDOUT/etc.) verbatim; a generic category is returned instead, with the raw detail preserved only in server-side logs.
  • A test demonstrates that registering N different webhooks and testing each once does not allow exceeding the intended aggregate test-request budget in a way that defeats the purpose of WEBHOOK_TEST_RATELIMIT_MAX.
  • Existing test/webhooks.routes.test.js continues to pass.
  • README's webhook section documents the reduced error detail exposed by the test endpoint and why.

Additional Notes

More precise references

  • src/routes/webhooks.js:142-156 (POST /webhooks/:id/test): confirmed exact response shape — delivery_id, status, attempts, response_status, last_error all returned directly to the caller, no filtering.
  • src/services/webhookDispatcher.js:37-44 (postOnce): confirmed validateStatus: () => true, meaning axios never rejects on a non-2xx HTTP response (only on genuine network-level failures) — this is what makes response_status a reliable "port open, got any HTTP response" signal distinct from a network-level rejection.
  • src/services/webhookDispatcher.js:180-191 (sendTest): confirmed sendTest calls deliverToWebhook(webhook, eventType, payload.event_id, payload) directly — the exact same function dispatch() uses for real pool.* events (line 145-153 area, deliverToWebhook defined at 145-153) — confirming the issue's claim that /webhooks/:id/test "shares dispatcher.deliverToWebhook under the hood."
  • src/routes/webhooks.js:21-25 (testLimit): confirmed built via buildRateLimit({ windowSeconds: config.webhooks.testRateLimit.windowSeconds, max: config.webhooks.testRateLimit.max, keyPrefix: 'webhooks_test' }) — a single shared keyPrefix across all test-endpoint calls from a given identifier, meaning the current fixed-window limiter (per Fixed-window rate limiter allows boundary bursts and trusts req.ip without configuring Express trust proxy #90) is keyed purely on identifier (IP), not on webhook id — confirming the issue's own concern that registering N webhooks and testing each once doesn't multiply the per-IP budget, but also confirming there's no separate per-webhook throttle layered on top today (the rate limit is already IP-scoped, not per-webhook-id-scoped, so "testing many different webhooks" doesn't currently bypass anything additional — it's already correctly bounded by the IP-level limiter; the issue's phrasing implies this should be double-checked/tested explicitly rather than assumed).
  • src/routes/webhooks.js:38-46 (validateCreate)/104-130 (PATCH /webhooks/:id): confirmed isValidUrl (lines 29-36) only checks protocol === 'http:' || 'https:' — no check against private/internal/link-local address ranges at all, at either creation or update time, confirming there is currently zero SSRF filtering anywhere in this router.

Additional edge cases

  • DNS rebinding: even if a companion SSRF fix validates the URL's hostname against private-IP ranges at registration/test time, axios.post(url, ...) in postOnce resolves DNS again at actual request time — a hostname that resolves to a public IP at validation time but is rebound to 127.0.0.1/an internal address by request time (attacker controls their own DNS) bypasses a naive one-time-validation approach entirely. The SSRF fix (tracked elsewhere, out of this issue's direct scope per its own text) needs either to validate the resolved IP at request time (not just the hostname at registration time) or use an HTTP client configured to reject redirects/re-resolutions to private ranges on every actual connection — worth flagging here since the test endpoint is the fastest, cheapest, most repeatable way an attacker would iterate on such a bypass, making this issue's hardening (reduced error detail, defense in depth) particularly relevant even after the primary SSRF fix lands.
  • IPv6 and unusual representations of loopback/private addresses (http://0x7f000001/, http://[::ffff:127.0.0.1]/, decimal/octal IP encodings) are common SSRF-filter-bypass techniques that a naive regex-based hostname check would miss — worth an explicit test matrix for these once the underlying SSRF-prevention issue is implemented, even though the actual validation logic itself is out of this issue's direct scope.
  • The suggestion to "return a generic 'delivery failed' externally" needs to be careful not to also break the legitimate debugging use case this endpoint exists for (a real subscriber testing their own real, external, non-internal webhook endpoint deserves to know their server returned a 500 vs. timed out, to fix their own integration) — the generic-error requirement should probably apply specifically when SSRF filtering itself is what rejected the request (a clear, distinct "target not allowed" error is fine and even helpful there), while genuine delivery failures against a legitimately-external URL could arguably keep somewhat more detail than "delivery failed" with nothing else — this nuance isn't addressed by the issue's current acceptance criteria and is worth raising explicitly in the PR/discussion rather than silently deciding one way.

Implementation sketch

  1. Once the companion SSRF-prevention validation function exists (e.g. isPublicAddress(hostname) or similar, resolving DNS and checking against RFC1918/loopback/link-local/multicast ranges), call it both at webhook create/update time (validateCreate, PATCH handler) AND at actual delivery time inside postOnce or immediately before it (to close the DNS-rebinding gap above) — sendTest's shared code path through deliverToWebhook means adding the check inside attempt()/postOnce automatically covers both real dispatches and test deliveries with one change.
  2. In src/routes/webhooks.js's test-endpoint handler, replace the direct passthrough of delivery.last_error with a generic mapping: last_error: delivery.last_error ? 'Delivery failed' : null (or a small enum of coarse categories — 'unreachable'/'error_response'/'timeout' — if slightly more granularity is desired without leaking raw ECONNREFUSED/ETIMEDOUT strings); keep the raw detail in the logger.warn/error calls already present in webhookDispatcher.js's attempt(), unchanged.
  3. Add a dedicated test (independent of any general SSRF test suite) specifically hitting POST /webhooks/:id/test with a webhook pre-registered against a private-range URL (bypassing create-time validation via direct repository seeding if needed, to simulate "what if create-time validation is ever bypassed/refactored") to prove the delivery-time check is what actually blocks it, not just the registration-time gate — this is exactly the "defense in depth against any SSRF-filter bypass" the issue asks for.

Test/reproduction plan

  • POST /webhooks/:id/test against a webhook whose URL resolves to 127.0.0.1, 169.254.169.254, and a private RFC1918 address (10.0.0.5) — each rejected with a clear, distinct "target not allowed" error, not attempted at all.
  • Same test but with the webhook's URL seeded directly into the repository bypassing route-level validation (simulating a bypass) — assert the delivery-time check in attempt()/postOnce still catches it.
  • response_status/last_error fields in the test endpoint's response, for a delivery against a real (mocked) unreachable/erroring target, no longer contain raw ECONNREFUSED/ETIMEDOUT-style strings — assert against a generic category instead.
  • Register 10 distinct webhooks pointing at 10 distinct (mocked, non-private) URLs and call /webhooks/:id/test once each from the same IP within one rate-limit window; assert the aggregate count against that IP's webhooks_test bucket reflects all 10 calls (i.e., confirm the existing IP-scoped limiter already correctly aggregates across different webhook ids, closing out the acceptance criterion about "testing each once does not allow exceeding the intended aggregate budget").

Cross-references

Metadata

Metadata

Assignees

No one assigned

    Labels

    GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial CampaignCampaign: Official CampaignOfficial Campaign | FWC26Campaign: Official Campaign | FWC26securitySecurity hardening and vulnerability fixesvery hardExtremely hard — deep expertise, careful design, and significant time requiredwebhooksWebhook delivery and notification

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions