cpp: model BDE bslx in-stream deserialization taint flow by kumarak · Pull Request #22453 · github/codeql

kumarak · 2026-08-27T16:59:00Z

Add flow summaries for the BDE byte-stream deserializers in BloombergLP::bslx:

ByteInStream and GenericInStream: constructing (or, for ByteInStream, resetting) from a buffer/streambuf taints the stream, and every get* method propagates that taint into its output variable. get* returns *this, so a fluent row keeps chained calls tainted.
InStreamFunctions::bdexStreamIn: the generic BDEX entry point that populates any bdex-compatible object from a stream, modeled as stream -> object.

Not a duplicate of the bsl.* container models or the std/bsl QL models, which cover only the bsl standard-library-alike types; the bslx BDE namespace had no coverage.

Copilot

Pull request overview

Adds C++ taint-flow models for Bloomberg BDE bslx deserialization.

Changes:

Models ByteInStream, GenericInStream, and bdexStreamIn.
Covers scalar, string, array, reset, and fluent-call flows.
Adds comprehensive external-model tests and release notes.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
`cpp/ql/lib/ext/bslx.model.yml`	Defines the new flow summaries.
`cpp/ql/test/library-tests/dataflow/external-models/bslx.cpp`	Exercises modeled APIs and chaining.
`cpp/ql/test/library-tests/dataflow/external-models/flow.expected`	Updates generated flow expectations.
`cpp/ql/test/library-tests/dataflow/external-models/steps.expected`	Updates generated summary-step expectations.
`cpp/ql/lib/change-notes/2026-08-27-bslx-models.md`	Documents the analysis improvement.

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

Add flow summaries for the BDE byte-stream deserializers in BloombergLP::bslx: - ByteInStream and GenericInStream<STREAMBUF>: constructing (or, for ByteInStream, reset-ing) from a buffer/streambuf taints the stream, and every get* method propagates that taint into its output variable. get* returns *this, so a fluent row keeps chained calls tainted. - InStreamFunctions::bdexStreamIn: the generic BDEX entry point that populates any bdex-compatible object from a stream, modeled as stream -> object. Not a duplicate of the bsl.* container models or the std/bsl QL models, which cover only the bsl standard-library-alike types; the bslx BDE namespace had no coverage. Verified with a BloombergLP::bslx-shaped stub in the dataflow external-models harness.

Drop scalar getter output summaries while retaining fluent stream flow. Use a type-aware QL model for bdexStreamIn object outputs to exclude scalars. Add regression coverage for integer outputs, fluent chaining, strings, and user-defined objects, and regenerate external-model expectations. Consolidate the review fixes and retain the merged main history. Validation: all five external-model tests pass without --learn; QL formatting and git diff --check pass.

jketema

Some comments. As before I've tried make some of the text shorter.

jketema · 2026-09-12T06:50:23Z

+	BloombergLP::bsls::Types::Uint64 ullbuf[16];
+	float fbuf[16];
+	double dbuf[16];
+	stream.getLength(i).getVersion(i).getInt8(c).getUint8(uc).getInt16(s).getUint16(us).getInt24(i).getUint24(ui).getInt32(i).getUint32(ui).getInt40(ll).getUint40(ull).getInt48(ll).getUint48(ull).getInt56(ll).getUint56(ull).getInt64(ll).getUint64(ull).getFloat32(f).getFloat64(d).getString(str).getArrayInt8(cbuf, 16).getArrayUint8(ucbuf, 16).getArrayInt16(sbuf, 16).getArrayUint16(usbuf, 16).getArrayInt24(ibuf, 16).getArrayUint24(uibuf, 16).getArrayInt32(ibuf, 16).getArrayUint32(uibuf, 16).getArrayInt40(llbuf, 16).getArrayUint40(ullbuf, 16).getArrayInt48(llbuf, 16).getArrayUint48(ullbuf, 16).getArrayInt56(llbuf, 16).getArrayUint56(ullbuf, 16).getArrayInt64(llbuf, 16).getArrayUint64(ullbuf, 16).getArrayFloat32(fbuf, 16).getArrayFloat64(dbuf, 16);


Ideally we should have a source and a sink call, so we can have a // $ ir annotation. There's no need to test this separately per function call.

jketema · 2026-09-12T06:51:34Z

+	BloombergLP::bsls::Types::Uint64 ullbuf[16];
+	float fbuf[16];
+	double dbuf[16];
+	stream.getLength(i).getVersion(i).getInt8(c).getUint8(uc).getInt16(s).getUint16(us).getInt24(i).getUint24(ui).getInt32(i).getUint32(ui).getInt40(ll).getUint40(ull).getInt48(ll).getUint48(ull).getInt56(ll).getUint56(ull).getInt64(ll).getUint64(ull).getFloat32(f).getFloat64(d).getString(str).getArrayInt8(cbuf, 16).getArrayUint8(ucbuf, 16).getArrayInt16(sbuf, 16).getArrayUint16(usbuf, 16).getArrayInt24(ibuf, 16).getArrayUint24(uibuf, 16).getArrayInt32(ibuf, 16).getArrayUint32(uibuf, 16).getArrayInt40(llbuf, 16).getArrayUint40(ullbuf, 16).getArrayInt48(llbuf, 16).getArrayUint48(ullbuf, 16).getArrayInt56(llbuf, 16).getArrayUint56(ullbuf, 16).getArrayInt64(llbuf, 16).getArrayUint64(ullbuf, 16).getArrayFloat32(fbuf, 16).getArrayFloat64(dbuf, 16);


Ideally we should have a source and a sink call, so we can have a // $ ir annotation. There's no need to test this separately per function call.

jketema · 2026-09-12T06:51:52Z

+---
+category: minorAnalysis
+---
+* Added taint flow summaries for the BDE `bslx` byte-stream deserializers `BloombergLP::bslx::ByteInStream`, `BloombergLP::bslx::GenericInStream`, and `BloombergLP::bslx::InStreamFunctions::bdexStreamIn`, so that data read from a `bslx` in-stream is tracked as tainted.


Suggested change

* Added taint flow summaries for the BDE `bslx` byte-stream deserializers `BloombergLP::bslx::ByteInStream`, `BloombergLP::bslx::GenericInStream`, and `BloombergLP::bslx::InStreamFunctions::bdexStreamIn`, so that data read from a `bslx` in-stream is tracked as tainted.

* Added taint flow summaries for the BDE `bslx` byte-stream deserializers `BloombergLP::bslx::ByteInStream`, `BloombergLP::bslx::GenericInStream`, and `BloombergLP::bslx::InStreamFunctions::bdexStreamIn`.

jketema · 2026-09-12T13:51:47Z

+private class BdexStreamIn extends TaintFunction {
+  BdexStreamIn() {
+    this.hasQualifiedName("BloombergLP::bslx::InStreamFunctions", "bdexStreamIn") and
+    this.getParameter(1).getUnspecifiedType().(ReferenceType).getBaseType().getUnspecifiedType()
+      instanceof Class
+  }
+
+  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
+    input.isParameterDeref(0) and
+    output.isParameterDeref(1)
+  }
+}


I prefer to more generic version in the yml file you had before, even though it does allow flow through scalar values. There is no particular harm in that flow. The difference with the other cases is that no explicit modelling is required here that bloats the yml file. In this case I would just trust the sanitizers in the queries to do their job, and if this causes performance problems somewhere down the line, then the better approach would be to extend the yml format so we can accurately model templated functions like these.

jketema · 2026-09-12T13:53:35Z

+      # === bslx::ByteInStream: concrete byte-array in-stream ===
+      # Taint in: the source buffer/streambuf taints the stream (`this`).


Second line is obvious from the definitions.

Suggested change

# === bslx::ByteInStream: concrete byte-array in-stream ===

# Taint in: the source buffer/streambuf taints the stream (`this`).

# bslx::ByteInStream

jketema · 2026-09-12T14:04:28Z

+      - ["BloombergLP::bslx", "ByteInStream", true, "getArrayUint64", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"]
+      - ["BloombergLP::bslx", "ByteInStream", true, "getArrayFloat32", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"]
+      - ["BloombergLP::bslx", "ByteInStream", true, "getArrayFloat64", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"]
+      # === bslx::GenericInStream<STREAMBUF>: streambuf-backed in-stream ===


Suggested change

# === bslx::GenericInStream<STREAMBUF>: streambuf-backed in-stream ===

# bslx::GenericInStream

jketema · 2026-09-12T14:04:50Z

+      - ["BloombergLP::bslx", "ByteInStream", true, "getArrayFloat32", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"]
+      - ["BloombergLP::bslx", "ByteInStream", true, "getArrayFloat64", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"]
+      # === bslx::GenericInStream<STREAMBUF>: streambuf-backed in-stream ===
+      # Taint in: the source buffer/streambuf taints the stream (`this`).


Suggested change

# Taint in: the source buffer/streambuf taints the stream (`this`).

jketema · 2026-09-12T14:05:26Z

+      # Taint out: the stream (`this`) taints the deserialized string/array output buffer.
+      # Scalar getters (getLength, getVersion, getInt*, getUint*, getFloat*) are deliberately
+      # not modeled as outputs: most queries sanitize taint through integers, so such rows
+      # would add nothing. Their fluent `ReturnValue[*]` rows below are still modeled.


Suggested change

# Taint out: the stream (`this`) taints the deserialized string/array output buffer.

# Scalar getters (getLength, getVersion, getInt*, getUint*, getFloat*) are deliberately

# not modeled as outputs: most queries sanitize taint through integers, so such rows

# would add nothing. Their fluent `ReturnValue[*]` rows below are still modeled.

# bslx::GenericInStream getters.

# Scalar getters (getLength, getVersion, getInt*, getUint*, getFloat*) are not modeled,

# as we are generally not interested in tainted scalar values.```

jketema · 2026-09-12T14:05:44Z

+      - ["BloombergLP::bslx", "GenericInStream", true, "getArrayUint64", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
+      - ["BloombergLP::bslx", "GenericInStream", true, "getArrayFloat32", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
+      - ["BloombergLP::bslx", "GenericInStream", true, "getArrayFloat64", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
+      # Fluent interface: each get* returns `*this`, keeping the returned stream tainted.


Suggested change

# Fluent interface: each get* returns `*this`, keeping the returned stream tainted.

# bslx::GenericInStream fluent interface

jketema · 2026-09-12T14:06:15Z

+      # === bslx::InStreamFunctions::bdexStreamIn: generic BDEX deserialization ===
+      # Free function template; `InStreamFunctions` is a namespace, so `type` is empty.
+      # Object outputs are modeled in implementations/Bslx.qll, where their type
+      # can be checked to exclude scalar outputs.


Suggested change

# === bslx::InStreamFunctions::bdexStreamIn: generic BDEX deserialization ===

# Free function template; `InStreamFunctions` is a namespace, so `type` is empty.

# Object outputs are modeled in implementations/Bslx.qll, where their type

# can be checked to exclude scalar outputs.

# bslx::InStreamFunctions::bdexStreamIn

kumarak requested a review from a team as a code owner August 27, 2026 16:59

Copilot AI balanced review requested due to automatic review settings August 27, 2026 16:59

Copilot started reviewing on behalf of kumarak August 27, 2026 16:59 View session

Copilot AI reviewed Aug 27, 2026

View reviewed changes

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

kumarak force-pushed the kumarak/cpp-bde-bslx-models branch from 77a4275 to ad27014 Compare August 30, 2026 20:39

jketema reviewed Aug 31, 2026

View reviewed changes

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

kumarak force-pushed the kumarak/cpp-bde-bslx-models branch from ad27014 to c7d50f6 Compare September 9, 2026 16:48

kumarak mentioned this pull request Sep 9, 2026

cpp: Model BDE bal codec taint flow #22537

Open

kumarak force-pushed the kumarak/cpp-bde-bslx-models branch from d889939 to ccff432 Compare September 12, 2026 12:38

kumarak requested a review from jketema September 12, 2026 12:39

jketema reviewed Sep 12, 2026

View reviewed changes

Navigation Menu

Uh oh!

cpp: model BDE bslx in-stream deserialization taint flow - #22453

cpp: model BDE bslx in-stream deserialization taint flow#22453
kumarak wants to merge 2 commits into
github:mainfrom
trail-of-forks:kumarak/cpp-bde-bslx-models

kumarak commented Aug 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

jketema left a comment

Uh oh!

jketema Sep 12, 2026

Uh oh!

jketema Sep 12, 2026

Uh oh!

jketema Sep 12, 2026

Uh oh!

jketema Sep 12, 2026

Uh oh!

jketema Sep 12, 2026

Uh oh!

jketema Sep 12, 2026

Uh oh!

jketema Sep 12, 2026

Uh oh!

jketema Sep 12, 2026

Uh oh!

jketema Sep 12, 2026

Uh oh!

jketema Sep 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	* Added taint flow summaries for the BDE `bslx` byte-stream deserializers `BloombergLP::bslx::ByteInStream`, `BloombergLP::bslx::GenericInStream`, and `BloombergLP::bslx::InStreamFunctions::bdexStreamIn`, so that data read from a `bslx` in-stream is tracked as tainted.
	* Added taint flow summaries for the BDE `bslx` byte-stream deserializers `BloombergLP::bslx::ByteInStream`, `BloombergLP::bslx::GenericInStream`, and `BloombergLP::bslx::InStreamFunctions::bdexStreamIn`.

		# === bslx::ByteInStream: concrete byte-array in-stream ===
		# Taint in: the source buffer/streambuf taints the stream (`this`).

	# === bslx::ByteInStream: concrete byte-array in-stream ===
	# Taint in: the source buffer/streambuf taints the stream (`this`).
	# bslx::ByteInStream

	# === bslx::GenericInStream<STREAMBUF>: streambuf-backed in-stream ===
	# bslx::GenericInStream

	# Fluent interface: each get* returns `*this`, keeping the returned stream tainted.
	# bslx::GenericInStream fluent interface

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!

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!

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