fix(channels): block SSRF via generic webhook URL (CWE-918)#318
Closed
sebastiondev wants to merge 1 commit into
Closed
fix(channels): block SSRF via generic webhook URL (CWE-918)#318sebastiondev wants to merge 1 commit into
sebastiondev wants to merge 1 commit into
Conversation
WebhookChannel.validate_webhook only enforced an https:// scheme, allowing the configured notification webhook to point at internal services such as cloud-metadata endpoints (169.254.169.254), loopback or RFC1918 addresses. push_notification() then issued an HTTPS POST to that destination, leaking metadata or pivoting into the internal network. Add an _is_public_host helper on the base channel that resolves the URL host and rejects loopback, link-local, private, multicast, reserved, unspecified and IPv4-mapped equivalents (covering all resolved A/AAAA records, not just the first). WebhookChannel uses the helper in both validate_webhook and as a defence-in-depth check in send(). Specific channels (Feishu, Slack, etc.) keep their existing allow-list and are unaffected.
This was referenced May 14, 2026
Contributor
Author
|
Closing this as inactive β no maintainer response after 14 days. The security finding and fix remain valid. If this is still relevant, I'm happy to reopen, rebase, or re-submit against a different branch. Just drop a comment. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #317
Summary
The generic
WebhookChannel(edict/backend/app/channels/webhook.py) validates a configured webhook URL only for scheme. Any user who can edit the notification webhook in the dashboard config can therefore point it at internal addresses β loopback, RFC1918, link-local (including cloud metadata at169.254.169.254) β and the server will dutifully POST the morning-brief payload to them on each scheduled push.This is a server-side request forgery primitive (CWE-918). It's bounded β the request method is fixed POST, body is a fixed JSON shape, and the existing scheme check already forces HTTPS so plain-HTTP IMDSv1 endpoints aren't directly hit β but it still gives an authenticated operator (or anyone, in default deployments without a dashboard password) a way to probe internal HTTPS services and to reach internal webhook receivers they otherwise have no network path to.
Affected:
WebhookChannel.validate_webhook/WebhookChannel.sendinedict/backend/app/channels/webhook.py.Data flow:
morning_brief_config.webhook(operator-controlled) βWebhookChannel.validate_webhook(scheme-only) βurlopen(Request(webhook, β¦))inWebhookChannel.send.Fix
Two small changes:
NotificationChannel._is_public_host(url)inedict/backend/app/channels/base.py. It resolves the URL host viasocket.getaddrinfoand rejects the URL if any resolved address is loopback, link-local, private, multicast, reserved, or unspecified β for both IPv4 and IPv6, including IPv4-mapped IPv6 (::ffff:127.0.0.1-style) which is a common bypass.WebhookChannel.validate_webhookcall both the scheme check and_is_public_host, and re-runvalidate_webhookinsidesend()as defence-in-depth against config tampering between validation and send.Rationale for putting the helper on the base class: other channels that accept user-controlled URLs in the future can opt in by calling the same helper, rather than each re-implementing the IP checks.
Tests
Added
tests/test_webhook_ssrf.pywith six cases (DNS mocked so the suite is hermetic):https://127.0.0.1,https://localhost) β rejectedhttps://169.254.169.254/...,https://metadata.google.internal/) β rejected10.0.0.5,192.168.1.1,172.16.0.1) β rejectedhttp://scheme β rejected (existing check)example.comβ93.184.216.34) β acceptedsend()to an unsafe URL never callsurlopenAll pass:
Security analysis
Preconditions to exploit:
morning_brief_config.webhookβ i.e. dashboard admin, or any unauthenticated user in deployments where no dashboard password is set (the default).What an attacker gains without the fix:
What the fix prevents:
Known limitation (worth flagging honestly): validation and
urlopeneach resolve DNS independently, so a perfectly-timed DNS rebind between the two resolutions could still slip past. Full rebinding immunity would require resolving once and connecting by the resolved IP (withHostheader preserved). Given the preconditions already require write-access to the webhook config, this residual risk is small; happy to follow up with a resolve-then-connect variant if you'd like.Adversarial review
Before submitting, we tried to disprove this. The main questions were: (a) does the existing HTTPS-only check already kill the interesting targets β no, it blocks HTTP IMDS endpoints but not internal HTTPS services or internal webhook receivers; (b) does requiring admin access make this redundant with what the admin can already do β dashboard admin grants config write but not arbitrary outbound POST capability from the server's network position, and in default deployments the dashboard has no password set at all, so the precondition is weak; (c) does any framework-level egress filter exist β there isn't one in the repo. So the finding stands.
Submitted by Sebastion β autonomous open-source security research from Foundation Machines. Free for public repos via the Sebastion AI GitHub App.