pubsub/awssnssqs: verify SNS envelope shape before trusting it - #3762
Merged
vangent merged 2 commits intoJul 31, 2026
Merged
Conversation
extractBody() treated any JSON body with a non-empty TopicArn field as an SNS notification, took its MessageAttributes as the message's Metadata, and replaced the body with its Message field. A producer that can only set the raw SQS message body (sqs:SendMessage, or any upstream that forwards attacker-influenced JSON into the queue) could forge that shape and inject arbitrary Metadata the subscriber would treat as coming from SNS, or silently swap out the delivered body. Require the fields SNS notifications actually carry (Type="Notification", MessageId, Timestamp, TopicArn) before unwrapping, and add SubscriptionOptions.SNSTopicARN (also settable via the snstopicarn URL parameter) so a subscription can pin the expected topic and reject envelopes claiming a different one.
…heck Each of the 3 added cases omits exactly one of the four required fields (Type, MessageId, Timestamp already covered structurally; here MessageId, Timestamp, and a wrong Type value), confirming extractBody actually requires all four rather than a subset that happens to pass the existing tests.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3762 +/- ##
=======================================
Coverage 75.38% 75.39%
=======================================
Files 104 104
Lines 14241 14250 +9
=======================================
+ Hits 10736 10744 +8
Misses 2768 2768
- Partials 737 738 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
2 participants
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.
extractBody()decides a raw SQS message body is an SNS notification by unmarshalling it as JSON and checking for a non-emptyTopicArnfield. If it matches, the envelope'sMessageAttributesbecome the delivered message'sMetadataand the body is replaced by the envelope'sMessagefield — nothing about the envelope's origin is checked.This runs for every
awssqs://subscription that doesn't setRaw: true(the default), including ones that only ever expect SNS-delivered traffic. Since SQS message bodies are attacker-influenced whenever anything upstream forwards user data into the queue (or the attacker simply hassqs:SendMessage), a body shaped like{"TopicArn":"anything","Message":"...","MessageAttributes":{...}}lets the sender choose theMetadatathe subscriber sees, or swap out the delivered body for something the real bytes never contained — despiteMetadatalooking like it was authoritatively set by SNS.This requires the shape an actual SNS notification has (
Type: "Notification",MessageId,Timestamp,TopicArn) before unwrapping, and addsSubscriptionOptions.SNSTopicARN(settable via thesnstopicarnURL parameter) so a subscription can additionally pin the topic it expects and ignore envelopes claiming a different one. Existing conformance tests (recorded against real SNS/SQS traffic) still pass unmodified since real notifications already have all four fields.