Repository navigation
fix(ses): verify AWS SNS signatures on the SES callback - #466
Open
polylane[bot] wants to merge 1 commit into
Open
polylane[bot] wants to merge 1 commit into
polylane[bot] wants to merge 1 commit into
Conversation
Co-authored-by: polylane[bot] <277585245+polylane[bot]@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying usesend with
|
| Latest commit: |
6b8aab8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://132fe0cc.usesend.pages.dev |
| Branch Preview URL: | https://polylane-autofix-p5ssbljl4ng.usesend.pages.dev |
This branch was successfully deployed
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.
Fixes: useSend SES callback accepts forged SNS events without signature verification
The public SES notification callback trusted any request that named a configured SNS topic ARN, because it never checked the AWS SNS message signature. A forged request could mark email as bounced or complained, unsubscribe contacts that share a recipient address, skew usage counters, and make the server fetch an attacker-controlled URL. The callback now verifies the SNS signature before it processes a message and only follows subscription URLs on the AWS SNS host.
flowchart TD A["POST /api/ses_callback"] --> B{"checkEventValidity"} B -->|"TopicArn not configured"| R1["401 rejected"] B -->|"TopicArn matches"| C{"verifySnsMessageSignature (new)"} C -->|"invalid or untrusted cert URL"| R1 C -->|"valid signature"| D{"Type == SubscriptionConfirmation"} D -->|"yes"| E{"isTrustedSnsUrl SubscribeURL (new)"} E -->|"untrusted"| R2["400 rejected, no fetch"] E -->|"trusted"| F["fetch SNS confirm URL, update setting"] D -->|"no"| G["SesHookParser.queue"]Detection evidence
Affected:
int_13ee3c91c001c2c8xfdj06a9Why this fix
The route is reachable in production and its only gate was a
TopicArnmembership check, which is an identifier rather than a secret. AWS requires an HTTPS SNS endpoint to verify the message signature before trusting the payload, and the unverified path reached the suppression and unsubscribe logic inses-hook-parser.ts. The change adds signature verification and host allowlists at the callback entry point. The one remaining item is optional:getTopicArnsstill returns every configured ARN, so scoping it per team stays open.Causal chain
5 files changed (+559/-12)
apps/web/src/app/api/ses_callback/route.api.test.ts: added, +156/-0apps/web/src/app/api/ses_callback/route.ts: modified, +30/-12apps/web/src/server/aws/sns-message-validator.ts: added, +177/-0apps/web/src/server/aws/sns-message-validator.unit.test.ts: added, +194/-0apps/web/src/types/aws-types.ts: modified, +2/-0Repository lint:
pnpm run lint(declared in package.json) fails over the whole repository (the declared target takes no file arguments), so the failure is not attributed to this change and did not block it; run it before merging.Lint output
Generated by Polylane. You can ask follow-ups by mentioning @polylane in a comment.
Summary by cubic
Verifies AWS SNS message signatures on the SES callback route so forged events are no longer trusted. Previously the route only checked that the TopicArn matched a configured value, so any request naming the right topic could trigger bounce/complaint handling, unsubscribe contacts, skew usage metrics, and fetch an attacker-controlled URL. The callback now verifies the SNS signature and only fetches SubscribeURL values pointing at the AWS SNS host for the message's region. Verification is still skipped in development.
Written for commit 6b8aab8. Summary will update on new commits.