Skip to content

codegen: escape string default values to prevent code injection#8964

Merged
jtdavis777 merged 2 commits intogoogle:masterfrom
KevinZhao:fix/escape-string-defaults
Mar 19, 2026
Merged

codegen: escape string default values to prevent code injection#8964
jtdavis777 merged 2 commits intogoogle:masterfrom
KevinZhao:fix/escape-string-defaults

Conversation

@KevinZhao
Copy link
Contributor

@KevinZhao KevinZhao commented Mar 7, 2026

Summary

String default values parsed from .fbs schemas are un-escaped by the IDL parser (e.g., \x22 becomes a raw " byte), but code generators embed these raw values directly into generated source code string literals without re-escaping. This allows specially crafted .fbs files to inject arbitrary code into generated C++, Rust, TypeScript, and Swift source files.

Changes

Apply EscapeString() to re-escape string default values at all 7 affected code generation sites across 5 generators:

  • C++ (idl_gen_cpp.cpp): bfbs_string literal + CreateString default
  • Rust (idl_gen_rust.cpp): string default value
  • TypeScript (idl_gen_ts.cpp): string default value
  • Swift (idl_gen_swift.cpp): string reader default + 2 constructor defaults
  • FBS (idl_gen_fbs.cpp): schema regeneration (strings only, not numeric defaults)

No new helpers introduced — all sites call flatbuffers::EscapeString() directly, per reviewer feedback.

Test plan

  • Build passes (cmake --build build)
  • Verified: \x22 in string defaults is correctly escaped to \" in generated C++ code
  • CI tests pass (existing tests use normal string defaults, unaffected by this change)

Resolves the TODO comments in idl_gen_cpp.cpp and idl_gen_rust.cpp.

@KevinZhao KevinZhao requested a review from dbaileychess as a code owner March 7, 2026 12:19
@github-actions github-actions bot added c++ javascript rust typescript codegen Involving generating code from schema swift labels Mar 7, 2026
@KevinZhao KevinZhao force-pushed the fix/escape-string-defaults branch from ade8648 to c5f531c Compare March 10, 2026 11:35
@jtdavis777
Copy link
Collaborator

hey @KevinZhao thank you for a fantastic PR and bug fix! This will take me a little time to ingest but I will run the workflows and plan to review and get merged.

String default values parsed from .fbs schemas are un-escaped by the IDL
parser (e.g., \x22 becomes a raw " byte), but code generators embed these
raw values directly into generated source code string literals. This allows
specially crafted .fbs files to break out of string literals and inject
arbitrary code into generated C++, Rust, TypeScript, and Swift source.

Fix by adding EscapeCodeGenString() helper that re-escapes string content
before embedding, and applying it to all 7 affected injection points across
5 code generators (C++, Rust, TypeScript, Swift, FBS).

Resolves the TODO comments in idl_gen_cpp.cpp and idl_gen_rust.cpp.
@KevinZhao KevinZhao force-pushed the fix/escape-string-defaults branch from c5f531c to cb9398a Compare March 12, 2026 02:48
@KevinZhao
Copy link
Contributor Author

Thanks @aardappel — good call. Removed EscapeCodeGenString entirely and now call EscapeString directly at each codegen site. Since EscapeString already produces a properly quoted string, the callers no longer need to add \"..\" manually.

The diff no longer touches util.h — all changes are in the 5 codegen files using the existing EscapeString API.

@KevinZhao
Copy link
Contributor Author

Hi @aardappel @jtdavis777 — I've addressed the review feedback (removed EscapeCodeGenString, now using EscapeString directly at each codegen site). The diff no longer touches util.h.

Happy to rebase on latest master if needed. Could you take another look when you get a chance?

@jtdavis777 jtdavis777 requested a review from aardappel March 19, 2026 01:47
@jtdavis777 jtdavis777 dismissed aardappel’s stale review March 19, 2026 01:48

issues were addressed by the PR creator -- will re-review myself :D

@jtdavis777
Copy link
Collaborator

jtdavis777 commented Mar 19, 2026

@KevinZhao LGTM - would you mind at your leisure updating your PR description? will merge this anyway but would be nice if the description was accurate

@jtdavis777 jtdavis777 merged commit 8afb68f into google:master Mar 19, 2026
51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants