⚡ Match Highlights: protocolbuffers/protobuf/pull/

thalha-a9 · 2026-07-18T17:38:55Z

Summary

Add input validation for language-specific namespace/package options (php_namespace, php_metadata_namespace, ruby_package, csharp_namespace) in their respective code generators.

These options are currently emitted directly into generated source code without character validation. Specially crafted .proto files can inject arbitrary code into generated PHP, Ruby, or C# output.

Fixes #28679

Changes

PHP generator: Validates php_namespace and php_metadata_namespace — denies injection-enabling characters (;, {}, (), quotes, $, etc.) while allowing Unicode identifiers
Ruby generator + RBS generator: Validates ruby_package — same denylist approach
C# generator: Validates csharp_namespace — same denylist approach

Each generator rejects invalid options with a clear error message before emitting code.

Motivation

When processing untrusted .proto files (e.g., from third-party dependencies, shared proto registries, or CI pipelines), these options can contain characters like semicolons, braces, or backticks that break out of the intended code structure. Validating at generation time prevents this class of injection.

Testing

The denied character sets match characters that can never be valid in a namespace/package identifier in any of the target languages. Unicode identifiers are allowed. Existing .proto files with valid namespace options are unaffected.

thalha-a9 · 2026-07-19T11:00:22Z

Hi @JasonLunn @bshaffer @amanda-tarafa - following up on this PR. Since I'm an external contributor, the pull_request_target checks (Tests / All Blocking Tests, Apply Safe for Tests Label) are failing because they need a maintainer to apply the "safe for tests" label before they can run.

Could one of you take a look and:

Apply the "safe for tests" label so the full test suite runs, and.
Review the change whenever you get a chance?

Happy to address any feedback. Thanks!

thalha-a9 · 2026-07-19T12:23:00Z

This PR addresses the issue described in #28679.

esrauchg

Thank your for the PR! Just one small adjustment recommended and then I think this will be good to go.

esrauchg · 2026-07-20T18:52:31Z

+    //  @ - verbatim strings
+    //  newlines - line injection
+    //  null bytes - string truncation
+    if (c == ';' || c == '{' || c == '}' || c == '(' || c == ')' ||


(I think I accidentally commented on the specific commit instead of the PR, so posting again here)

Nit: I think
constexpr absl::CharSet kBannedBackageChars(";{}()'"`$# ... ");

And then just

if (kBannedBackageChars.contains(c))

(Same comment for other files)

Done — switched all three validators to use constexpr absl::CharSet. Much cleaner, thanks for the suggestion.

esrauchg · 2026-07-20T18:53:18Z

+      "protocol_compiler --proto_path=$tmpdir --ruby_out=$tmpdir foo.proto");
+
+  // Newlines in the option value should be rejected (either by protoc parser
+  // or by our validator if they arrive as literal \n bytes).


Was there a missing test here?

Good catch, yeah that was missing. Added ExpectErrorSubstring for it now.

thalha-a9 · 2026-07-21T01:42:06Z

Updated — both changes addressed in the latest push:

Switched to constexpr absl::CharSet across all three generators (C#, PHP, Ruby)
Added the missing ExpectErrorSubstring assertion in the Ruby newline test

Let me know if anything else needs adjusting.

esrauchg

Just one further nit and then this is good to go. Thanks for the contribution!

esrauchg · 2026-07-21T13:08:50Z

+// to allow Unicode identifiers while blocking dangerous characters.
+bool IsValidCSharpNamespace(absl::string_view ns, std::string* error) {
+  for (char c : ns) {
+    if (c == '\0' || kBannedNamespaceChars.contains(c)) {


You pulled off \0 because its annoying to embed one in a char* literal, I think you can actually write it like this though:

constexpr absl::CharSet kBannedNamespaceChars =
absl::CharSet(";{}()'"`$#=<>|&[]/\@\n\r!?*~^%,") | absl::CharSet::Char('\0');

(In modern C++ you could also do it with a stringview literal which works cleanly with embedded \0, like "a\0b"sv, but I think we avoid those in our style guide)

Nice, didn't know about the union operator on CharSet. Updated all three to use it.

thalha-a9 · 2026-07-21T13:35:05Z

Updated — using absl::CharSet::Char('\0') with the union operator now so everything lives in the CharSet. Pushed as a single clean commit.

esrauchg

Thanks!

thalha-a9 · 2026-07-21T14:12:20Z

Hi @esrauchg, thanks for the approval! Could the safe for tests label be added so CI can run? Just want to make sure everything's green before merge. Appreciate it!

thalha-a9 · 2026-07-21T15:15:41Z

Hi @esrauchg, I've pushed a fix adding the missing
@abseil-cpp//absl/strings:charset dependency to the cc_library targets
for ruby, csharp, and php_generator. Could you re-apply the "safe for
tests" label so CI can run on the latest commit? Thanks!

thalha-a9 · 2026-07-22T01:04:43Z

All checks passing now, ready to merge whenever you get a chance.

thalha-a9 · 2026-07-25T08:50:28Z

@esrauchg Could you also apply the safe for tests label so the CI checks can pass? That's the only thing blocking merge right now.

thalha-a9 · 2026-07-26T09:04:57Z

@esrauchg Friendly ping — this has been approved and CI is green for a week now. Could you merge when you get a chance?

@mkruskal-google @haberman Tagging in case esrauchg is unavailable — PR is approved, all checks passing, just needs someone to hit merge. Thanks!

thalha-a9 · 2026-07-31T14:56:39Z

Hi @bshaffer and @shaod2, I hope you are both doing well. I wanted to respectfully follow up on this pull request. The review has been approved and all CI checks are passing, so I believe everything is in order for a merge. I completely understand how busy things can get, and I truly appreciate your time. Please do let me know if there is anything further you need from me.

esrauchg · 2026-07-31T15:29:18Z

Sorry I was out of office but am now back. Unfortunately our github flow is kind of sucky (for 'good reasons') and stuff gets stuck sometimes.

Just for 'how the sausage is made is made', we actually have a separate internal repo that is our source of truth for this code (and other pieces of code that aren't necessarily all open sourced). The commit flow when we accept external PRs is that the PR gets into a state that a bot will pick up and create a second internal PR (which also does things like run additional internal only tests that aren't in the external repo), and after that PR is committed it will sync back out and then the original PR marked closed.

It looks like this maybe is stuck because the branch has a conflict, can you merge?

thalha-a9 · 2026-07-31T15:41:03Z

Hey @esrauchg Noticed this branch now shows a merge conflict with main (in src/google/protobuf/compiler/php/generator_unittest.cc) looks like main added a new Edition2026Fails test right next to the namespace-validation tests this PR adds. No actual logic conflict, just adjacent insertions. Happy to rebase and keep both blocks. let me know if you'd prefer I do that now.

esrauchg · 2026-07-31T15:43:35Z

Yeah, do you mind fixing the conflicts?

The merge conflict showed up because I clicked a button to attempt rebase (which is a behavior that PRs sent to us opt into letting maintainers of our repo do to your fork that you're sending the PR from, usually it goes cleanly and doesn't need a manual touch).

Add input validation for language-specific namespace/package options (php_namespace, php_metadata_namespace, ruby_package, csharp_namespace) in their respective code generators. These options are currently emitted directly into generated source code without character validation. Specially crafted .proto files could inject arbitrary code into generated PHP, Ruby, or C# output. Each generator validates its namespace option using a constexpr absl::CharSet denylist of injection-enabling characters, rejecting invalid input with a clear error message before emitting code. A denylist (rather than an ASCII allowlist) is used so that Unicode identifiers, common in some language ecosystems, remain supported while still blocking dangerous characters. - PHP generator: validates php_namespace and php_metadata_namespace - Ruby generator + RBS generator: validates ruby_package - C# generator: validates csharp_namespace Unit tests added for all three generators verifying rejection of malicious input and acceptance of valid namespaces.

thalha-a9 · 2026-07-31T15:50:52Z

@esrauchg Rebased on current main and resolved the conflict in generator_unittest.cc Branch is up to date and pushed to my fork, should show as mergeable now. Could you also apply the "safe for tests" label so CI can run?

thalha-a9 · 2026-07-31T16:21:27Z

@esrauchg approval and checks are all green now. This should be ready to merge whenever you get a chance.

## Summary Add input validation for language-specific namespace/package options (`php_namespace`, `php_metadata_namespace`, `ruby_package`, `csharp_namespace`) in their respective code generators. These options are currently emitted directly into generated source code without character validation. Specially crafted `.proto` files can inject arbitrary code into generated PHP, Ruby, or C# output. Fixes #28679 ## Changes - **PHP generator**: Validates `php_namespace` and `php_metadata_namespace` — denies injection-enabling characters (`;`, `{}`, `()`, quotes, `$`, etc.) while allowing Unicode identifiers - **Ruby generator + RBS generator**: Validates `ruby_package` — same denylist approach - **C# generator**: Validates `csharp_namespace` — same denylist approach Each generator rejects invalid options with a clear error message before emitting code. ## Motivation When processing untrusted `.proto` files (e.g., from third-party dependencies, shared proto registries, or CI pipelines), these options can contain characters like semicolons, braces, or backticks that break out of the intended code structure. Validating at generation time prevents this class of injection. ## Testing The denied character sets match characters that can never be valid in a namespace/package identifier in any of the target languages. Unicode identifiers are allowed. Existing `.proto` files with valid namespace options are unaffected. Closes #28667 COPYBARA_INTEGRATE_REVIEW=#28667 from thalha-a9:fix/validate-codegen-namespace-options b2f333a FUTURE_COPYBARA_INTEGRATE_REVIEW=#28667 from thalha-a9:fix/validate-codegen-namespace-options b2f333a PiperOrigin-RevId: 957188071

php_class_prefix is pasted directly into `class <prefix><Name> extends ...`, so a prefix containing `{`, `}` or `;` can close the generated declaration and open an attacker-defined one. The result is still valid PHP and the real message class is still emitted, so nothing fails to build. protocolbuffers#28667 added this validation for php_namespace and php_metadata_namespace but not for php_class_prefix, which is read from names.cc rather than php_generator.cc. IsValidPhpNamespace becomes IsValidPhpOption so the error names the offending option. The message still begins "Invalid character", so the tests added in protocolbuffers#28667 are unaffected.

## Summary Add input validation for language-specific namespace/package options (`php_namespace`, `php_metadata_namespace`, `ruby_package`, `csharp_namespace`) in their respective code generators. These options are currently emitted directly into generated source code without character validation. Specially crafted `.proto` files can inject arbitrary code into generated PHP, Ruby, or C# output. Fixes #28679 ## Changes - **PHP generator**: Validates `php_namespace` and `php_metadata_namespace` — denies injection-enabling characters (`;`, `{}`, `()`, quotes, `$`, etc.) while allowing Unicode identifiers - **Ruby generator + RBS generator**: Validates `ruby_package` — same denylist approach - **C# generator**: Validates `csharp_namespace` — same denylist approach Each generator rejects invalid options with a clear error message before emitting code. ## Motivation When processing untrusted `.proto` files (e.g., from third-party dependencies, shared proto registries, or CI pipelines), these options can contain characters like semicolons, braces, or backticks that break out of the intended code structure. Validating at generation time prevents this class of injection. ## Testing The denied character sets match characters that can never be valid in a namespace/package identifier in any of the target languages. Unicode identifiers are allowed. Existing `.proto` files with valid namespace options are unaffected. Closes #28667 COPYBARA_INTEGRATE_REVIEW=#28667 from thalha-a9:fix/validate-codegen-namespace-options b2f333a PiperOrigin-RevId: 957224133

…29130) ## Summary Follow-up to #28667, which added character validation for `php_namespace`, `php_metadata_namespace`, `ruby_package` and `csharp_namespace` because they are emitted directly into generated source without validation. Two options in the same family were not covered and are still unvalidated on `main`: | option | read at | validated before this PR | |---|---|---| | `php_class_prefix` | `php/names.cc` (`ClassNamePrefixImpl`) | no | | `objc_class_prefix` | `objectivec/names.cc` (`FileClassPrefix`) | no | `php_class_prefix` is easy to miss by inspection: the guard added in #28667 and both of its call sites are in `php_generator.cc`, while `php_class_prefix` is read from `names.cc`. ## What happens today **PHP.** The generator emits `class <php_class_prefix><Name> extends ...`, so a prefix that closes the declaration and opens its own is emitted verbatim: ```proto option php_class_prefix = "Pfx{} class Injected { public static function pwn() { return 'INJECTED'; } } class "; ``` `protoc --php_out` exits 0 and the generated file contains: ```php class Pfx{} class Injected { public static function pwn() { return 'INJECTED'; } } class M extends \Google\Protobuf\Internal\Message ``` That is valid PHP (`php -l`: no syntax errors), the injected class is defined and its code runs, and `Tp\M` is still generated correctly — so nothing fails to build and nothing looks wrong. **Objective-C.** `FileClassPrefix` returns the option verbatim, so with `objc_class_prefix = "A\";void pwn(){}//"` the value lands in `@interface` / `@implementation` declarations, a `typedef struct` tag and the `.prefix = "..."` string literal. I have not built the resulting Objective-C, so I am claiming unvalidated emission here rather than a working payload. Worth noting separately: a prefix check already exists, but it validates style rather than characters, so it warns about a legitimate short prefix and says nothing about a hostile one: ``` objc_class_prefix = "AB" -> warning: Invalid 'option objc_class_prefix = "AB";' objc_class_prefix = "A\";void pwn(){}//" -> no diagnostic at all ``` ## Changes - **PHP generator** — generalised `IsValidPhpNamespace` to `IsValidPhpOption(value, option_name, error)` so the error names the offending option, and applied it to `php_class_prefix` alongside the two existing options. The message still begins `Invalid character`, so the tests added in #28667 are unaffected. - **Objective-C generator** — new `ValidateObjCClassPrefixChars()` rejects prefixes containing anything outside `[A-Za-z0-9_]`, run from `ValidateObjCClassPrefixes()` over every file ahead of the expected-prefixes opt-outs. - Tests for both, in the reject/accept pattern used by #28667. ## Notes for review Three things I'd rather flag than have you find: 1. **The Objective-C check is an allowlist, not a denylist.** The prefix is pasted into C identifiers, where nothing outside `[A-Za-z0-9_]` is ever valid, so an allowlist closes the class rather than chasing characters. I checked every `objc_class_prefix` in this repo (including the deliberately-empty one) and none would be rejected. Happy to switch to the `absl::CharSet` denylist used for PHP/Ruby/C# if you'd prefer consistency over precision. 2. **Placement matters.** The character check runs in `ValidateObjCClassPrefixes()` over every file, ahead of both documented opt-outs (`expected_prefixes_path=-` and `expected_prefixes_suppressions`), since disabling the naming *policy* should not disable a correctness check. There is a regression test per opt-out. 3. **Java is gated, but to a much narrower bar** — `java/file.cc` rejects only `;`, `\r`, `\n` and space, and its comment says it checks "only a subset". Since Java accepts `/**/` as a whitespace substitute, `java_outer_classname = "Evil/**/implements/**/Cloneable"` and `"Evil/*"` are both emitted into the class declaration on current `main`. I could not get code execution out of that (`;` blocks statement injection); the demonstrated effect is generated code that will not compile. **Not changed in this PR** — mentioning it in case you want the bar aligned, and happy to do it in a follow-up. Also, for whatever it's worth to release planning: #28667 itself is not in any released binary yet — the validation string is absent from both `protoc 35.1` and `protoc 36.0-rc2`, since it landed after the v36 branch was cut. ## Testing `src/google/protobuf/compiler/php/generator_unittest.cc` — `InvalidPhpClassPrefixRejected`, `ValidPhpClassPrefixAccepted`. `src/google/protobuf/compiler/objectivec/generator_unittest.cc` (new file, plus its `cc_test` target) — `InvalidObjCClassPrefixRejected`, `InvalidObjCClassPrefixRejectedWithExpectedPrefixesDisabled`, `InvalidObjCClassPrefixRejectedWhenSuppressed`, `ValidObjCClassPrefixAccepted`, `ObjCClassPrefixWithUnderscoreAccepted`. The two opt-out tests fail with the character check in its original location inside `ValidateObjCClassPrefix()`, and pass with it moved. Both directions are covered, so the tests fail if the validation is removed *and* if it is made too strict. --- _Both tests were run locally against this branch (`bazel test //src/google/protobuf/compiler/php:generator_unittest //src/google/protobuf/compiler/objectivec:generator_unittest`), and re-run with the two guards reverted to confirm they fail without the fix._ Closes #29130 COPYBARA_INTEGRATE_REVIEW=#29130 from BarakSrour:validate-class-prefix-options a2bbd8f PiperOrigin-RevId: 966161028

thalha-a9 requested review from a team as code owners July 18, 2026 17:38

thalha-a9 requested review from JasonLunn, amanda-tarafa and bshaffer and removed request for a team July 18, 2026 17:38

thalha-a9 force-pushed the fix/validate-codegen-namespace-options branch from 6a395e9 to cb897ec Compare July 19, 2026 12:21

thalha-a9 mentioned this pull request Jul 19, 2026

Code injection via unvalidated namespace options in code generators #28679

Closed

thalha-a9 force-pushed the fix/validate-codegen-namespace-options branch from bdd46d1 to ce42e58 Compare July 20, 2026 16:26

esrauchg requested changes Jul 20, 2026

View reviewed changes

thalha-a9 force-pushed the fix/validate-codegen-namespace-options branch from d125fa7 to ea270ce Compare July 21, 2026 01:41

esrauchg reviewed Jul 21, 2026

View reviewed changes

thalha-a9 force-pushed the fix/validate-codegen-namespace-options branch from ea270ce to af1e496 Compare July 21, 2026 13:34

thalha-a9 requested a review from esrauchg July 21, 2026 13:35

esrauchg approved these changes Jul 21, 2026

View reviewed changes

thalha-a9 requested a review from esrauchg July 21, 2026 14:08

esrauchg added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jul 21, 2026

github-actions Bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jul 21, 2026

thalha-a9 force-pushed the fix/validate-codegen-namespace-options branch from af1e496 to 4144598 Compare July 21, 2026 15:06

esrauchg added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jul 21, 2026

github-actions Bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jul 21, 2026

shaod2 added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jul 21, 2026

github-actions Bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jul 21, 2026

shaod2 added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jul 21, 2026

github-actions Bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jul 21, 2026

thalha-a9 requested a review from bshaffer July 23, 2026 14:38

esrauchg approved these changes Jul 31, 2026

View reviewed changes

thalha-a9 force-pushed the fix/validate-codegen-namespace-options branch from 3856b6e to b2f333a Compare July 31, 2026 15:47

esrauchg added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jul 31, 2026

esrauchg approved these changes Jul 31, 2026

View reviewed changes

github-actions Bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jul 31, 2026

copybara-service Bot mentioned this pull request Jul 31, 2026

Validate language-specific namespace options in code generators (#28667) #28953

Merged

copybara-service Bot closed this in 15de228 Jul 31, 2026

BarakSrour mentioned this pull request Aug 11, 2026

Validate php_class_prefix and objc_class_prefix in code generators #29130

Closed

bshaffer mentioned this pull request Oct 13, 2026

Harden PHP repeated field indexing and namespace option validation #27852

Closed

Conversation

thalha-a9 commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Motivation

Testing

Uh oh!

thalha-a9 commented Jul 19, 2026

Uh oh!

thalha-a9 commented Jul 19, 2026

Uh oh!

esrauchg left a comment

Choose a reason for hiding this comment

Uh oh!

esrauchg Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

thalha-a9 Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

esrauchg Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

thalha-a9 Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

thalha-a9 commented Jul 21, 2026

Uh oh!

esrauchg left a comment

Choose a reason for hiding this comment

Uh oh!

esrauchg Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

thalha-a9 Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

thalha-a9 commented Jul 21, 2026

Uh oh!

esrauchg left a comment

Choose a reason for hiding this comment

Uh oh!

thalha-a9 commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thalha-a9 commented Jul 21, 2026

Uh oh!

thalha-a9 commented Jul 22, 2026

Uh oh!

thalha-a9 commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thalha-a9 commented Jul 26, 2026

Uh oh!

thalha-a9 commented Jul 31, 2026

Uh oh!

esrauchg commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thalha-a9 commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

esrauchg commented Jul 31, 2026

Uh oh!

thalha-a9 commented Jul 31, 2026

Uh oh!

thalha-a9 commented Jul 31, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

thalha-a9 commented Jul 18, 2026 •

edited

Loading

thalha-a9 commented Jul 21, 2026 •

edited

Loading

thalha-a9 commented Jul 25, 2026 •

edited

Loading

esrauchg commented Jul 31, 2026 •

edited

Loading

thalha-a9 commented Jul 31, 2026 •

edited

Loading