Skip to content

escape attacker-controlled strings in the json encoder#28678

Open
arshsmith1 wants to merge 1 commit into
protocolbuffers:mainfrom
arshsmith1:json-encode-escape-strings
Open

escape attacker-controlled strings in the json encoder#28678
arshsmith1 wants to merge 1 commit into
protocolbuffers:mainfrom
arshsmith1:json-encode-escape-strings

Conversation

@arshsmith1

Copy link
Copy Markdown
Contributor

jsonenc_fieldpath only rejects uppercase letters and malformed underscores, then writes every other byte straight into the output with jsonenc_putbytes, inside the quotes that jsonenc_fieldmask puts around it. FieldMask paths are ordinary wire data, so a peer who sends the path a","injected":"b gets {"maskVal":"a","injected":"b"} out of upb_JsonEncode and can add top-level keys of their choosing. jsonenc_fieldval has the same shape for field names: json_name is the one name the def builder never runs through _upb_DefBuilder_CheckIdent, so a descriptor carrying json_name = "a\":1,\"b" also breaks out of the string.

The fix follows what WriteFieldMask in src/google/protobuf/json/internal/unparser.cc already does, which is an allowlist of [a-z0-9._] and an error on anything else, so the two implementations agree on which masks are encodable. Field names go through jsonenc_string for the same reason C++ routes them through WriteEscapedUtf8. Both belong in the encoder rather than in the callers, since the callers here are just handing over message data they parsed. Tests added for each; they fail on main and pass with this change, and the upb suite including conformance stays green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant