cpp: model Protocol Buffers parse/serialize taint flow by kumarak · Pull Request #22448 · github/codeql

kumarak · 2026-08-27T15:30:42Z

Add flow summaries for the protobuf C++ API on google::protobuf::MessageLite (subtypes=true, so Message and all generated messages are covered):

ParseFrom*/MergeFrom* (string, array, Cord, istream, zero-copy and coded-stream forms) propagate taint from the encoded input to the message.
SerializeTo*/AppendTo* propagate taint from the message to the output buffer or stream; SerializeAs*/AppendTo* to the return value.
File-descriptor variants are omitted (the fd is an int, not a buffer).

Add flow summaries for the protobuf C++ API on google::protobuf::MessageLite (subtypes=true, so Message and all generated messages are covered): - ParseFrom*/MergeFrom* (string, array, Cord, istream, zero-copy and coded-stream forms) propagate taint from the encoded input to the message. - SerializeTo*/AppendTo* propagate taint from the message to the output buffer or stream; SerializeAs*/... to the return value. File-descriptor variants are omitted (the fd is an int, not a buffer).

Copilot

Pull request overview

Adds C++ taint-flow summaries for Protocol Buffers MessageLite APIs and inherited generated message types.

Changes:

Models parse/merge and serialization flows.
Adds representative flow tests and expected results.
Documents the analysis improvement.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`cpp/ql/lib/ext/Protobuf.model.yml`	Defines protobuf flow summaries.
`cpp/ql/test/library-tests/dataflow/external-models/protobuf.cpp`	Adds protobuf test fixtures.
`cpp/ql/test/library-tests/dataflow/external-models/flow.expected`	Updates flow expectations.
`cpp/ql/test/library-tests/dataflow/external-models/steps.expected`	Updates summary-step expectations.
`cpp/ql/lib/change-notes/2026-08-27-protobuf-models.md`	Records the new models.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+      - ["google::protobuf", "MessageLite", True, "ParseFromZeroCopyStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
+      - ["google::protobuf", "MessageLite", True, "ParsePartialFromZeroCopyStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
+      - ["google::protobuf", "MessageLite", True, "ParseFromBoundedZeroCopyStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
+      - ["google::protobuf", "MessageLite", True, "ParsePartialFromBoundedZeroCopyStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
+      - ["google::protobuf", "MessageLite", True, "MergeFromBoundedZeroCopyStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]


jketema

Thanks for this. I made a brief first pass over this, which should hopefully put you on the right path.

Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com>

jketema

Some further comments. I think the .yml file looks good now. I would still significantly reduce the number of comments, which don't seem to add much.

jketema · 2026-08-31T09:01:41Z

+---
+category: minorAnalysis
+---
+* Added flow summaries for the Protocol Buffers C++ API (`google::protobuf::MessageLite`, covering `Message` and all generated messages).


Suggested change

* Added flow summaries for the Protocol Buffers C++ API (`google::protobuf::MessageLite`, covering `Message` and all generated messages).

* Added flow summaries for the Protocol Buffers `google::protobuf::MessageLite` C++ API.

jketema · 2026-08-31T09:03:06Z

+      # Flow summaries for the Protocol Buffers C++ API. All of these methods are declared on
+      # `google::protobuf::MessageLite`; `subtypes` covers `Message` and every generated message.
+      #
+      # File-descriptor variants (`{Parse,Serialize}*FromFileDescriptor`) are intentionally omitted:
+      # the descriptor is an `int`, not a data buffer, so there is no buffer argument to model.


Suggested change

# Flow summaries for the Protocol Buffers C++ API. All of these methods are declared on

# `google::protobuf::MessageLite`; `subtypes` covers `Message` and every generated message.

#

# File-descriptor variants (`{Parse,Serialize}*FromFileDescriptor`) are intentionally omitted:

# the descriptor is an `int`, not a data buffer, so there is no buffer argument to model.

# File-descriptor variants (`{Parse,Serialize}*FromFileDescriptor`) are intentionally omitted:

# the descriptor is an `int`, not a data buffer, so there is no buffer argument to model.

jketema · 2026-08-31T09:04:20Z

+      # Deserialization: the encoded input taints the message (`this`). The `*FromString` methods each
+      # have a `string_view` overload (the buffer is the by-value argument, so `Argument[0]`) and a
+      # `const Cord &` overload (the buffer is behind a reference, so `Argument[*0]`). The remaining
+      # inputs below are pointers or references, so they take `Argument[*0]`.


Suggested change

# Deserialization: the encoded input taints the message (`this`). The `*FromString` methods each

# have a `string_view` overload (the buffer is the by-value argument, so `Argument[0]`) and a

# `const Cord &` overload (the buffer is behind a reference, so `Argument[*0]`). The remaining

# inputs below are pointers or references, so they take `Argument[*0]`.

# Deserialization

jketema · 2026-08-31T09:05:01Z

+      - ["google::protobuf", "MessageLite", True, "MergeFromCodedStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
+      - ["google::protobuf", "MessageLite", True, "MergePartialFromCodedStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
+
+      # Serialization into an output buffer/stream: the message (`this`) taints `Argument[*0]`.


Suggested change

# Serialization into an output buffer/stream: the message (`this`) taints `Argument[*0]`.

# Serialization

jketema · 2026-08-31T09:05:28Z

+      - ["google::protobuf", "MessageLite", True, "SerializeToCodedStream", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
+      - ["google::protobuf", "MessageLite", True, "SerializePartialToCodedStream", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
+
+      # Serialization returning the bytes: the message (`this`) taints the (by-value) return value.


Suggested change

# Serialization returning the bytes: the message (`this`) taints the (by-value) return value.

# Serialization returning bytes

jketema · 2026-08-31T09:06:41Z

+}
+
+namespace absl {
+	// `absl::string_view` is passed by value; `absl::Cord` is passed by const reference.


This is completely out-of-context here. I'd just remove it.

Suggested change

// `absl::string_view` is passed by value; `absl::Cord` is passed by const reference.

jketema · 2026-08-31T09:08:14Z

+	// A faithful subset of `MessageLite`. The string/Cord/stream signatures mirror the real
+	// `message_lite.h`; the iostream-based methods are declared on `Message` in the real headers
+	// but are modeled here on `MessageLite` (with `subtypes` covering `Message`).


This read like a shortcut was taken that does not accurately represent actual protobuf. This should be fixed.

jketema · 2026-08-31T09:11:57Z

+
+// Every modeled method is called below so its summary step is covered by `steps.ql`. Endpoint
+// mistakes and rows that fail to bind show up as missing lines in `steps.expected`.
+void test_step_coverage() {


None of the "tests" below tell me that any of this is actually working. Ideally there should be sink calls here with // $ ir annotations.

kumarak requested a review from a team as a code owner August 27, 2026 15:30

Copilot AI balanced review requested due to automatic review settings August 27, 2026 15:30

Copilot started reviewing on behalf of kumarak August 27, 2026 15:31 View session

Copilot AI reviewed Aug 27, 2026

View reviewed changes

jketema requested changes Aug 28, 2026

View reviewed changes

Comment thread cpp/ql/lib/ext/Protobuf.model.yml Outdated

Comment thread cpp/ql/test/library-tests/dataflow/external-models/protobuf.cpp Outdated

Comment thread cpp/ql/lib/change-notes/2026-08-27-protobuf-models.md Outdated

github-actions Bot added C++ documentation labels Aug 28, 2026

kumarak and others added 2 commits August 28, 2026 12:28

Update cpp/ql/lib/change-notes/2026-08-27-protobuf-models.md

5f0b3df

Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com>

fix(cpp): correct protobuf string parse models and expand test coverage

d0b3dd5

jketema reviewed Aug 31, 2026

View reviewed changes

Navigation Menu

Uh oh!

cpp: model Protocol Buffers parse/serialize taint flow - #22448

cpp: model Protocol Buffers parse/serialize taint flow#22448
kumarak wants to merge 3 commits into
github:mainfrom
trail-of-forks:kumarak/cpp-protobuf-flow-models

kumarak commented Aug 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

jketema left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jketema left a comment

Uh oh!

jketema Aug 31, 2026

Uh oh!

jketema Aug 31, 2026

Uh oh!

jketema Aug 31, 2026

Uh oh!

jketema Aug 31, 2026

Uh oh!

jketema Aug 31, 2026

Uh oh!

jketema Aug 31, 2026

Uh oh!

jketema Aug 31, 2026

Uh oh!

jketema Aug 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	* Added flow summaries for the Protocol Buffers C++ API (`google::protobuf::MessageLite`, covering `Message` and all generated messages).
	* Added flow summaries for the Protocol Buffers `google::protobuf::MessageLite` C++ API.

	# Serialization into an output buffer/stream: the message (`this`) taints `Argument[*0]`.
	# Serialization

	# Serialization returning the bytes: the message (`this`) taints the (by-value) return value.
	# Serialization returning bytes

Uh oh!

Conversation

kumarak commented Aug 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

jketema left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jketema left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants