Skip to content

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

Open
kumarak wants to merge 2 commits into
github:mainfrom
trail-of-forks:kumarak/cpp-bde-bslx-models
Open

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

Conversation

@kumarak

@kumarak kumarak commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

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.

@kumarak
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 AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread cpp/ql/lib/ext/bslx.model.yml Outdated
- ["BloombergLP::bslx", "ByteInStream", true, "getInt16", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
- ["BloombergLP::bslx", "ByteInStream", true, "getUint16", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
- ["BloombergLP::bslx", "ByteInStream", true, "getInt24", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
- ["BloombergLP::bslx", "ByteInStream", true, "getUint24", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]

@jketema jketema Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generally do not define taint tracking models where the output in an integer, as in most of our queries we sanitize taint flow through integers. So, I think we should drop models like these (not the the models that specify ReturnValue[*] as output though). Unless you have a specific use-case for them.

C/C++ has a slight exception: for value preserving dataflow, we might want to have relevant summary models, as this would allow flag up more array out-of-bounds issues. However, that does not apply here as the best you can have here is taint.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jketema for the feedback. I dropped all summaries whose output is scalar, including getLength, getVersion, getInt*/getUint*, and getFloat*, since the same reasoning applies. The ReturnValue[*] rows and string/array outputs remain. I also added no-flow cases for getInt32 to document this.

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.
@kumarak
kumarak force-pushed the kumarak/cpp-bde-bslx-models branch from ad27014 to c7d50f6 Compare September 9, 2026 16:48
Remove the taint summaries whose output is an integer or float
(getLength, getVersion, getInt*, getUint*, getFloat*) on ByteInStream
and GenericInStream. Most queries sanitize taint through integers, so
these rows add nothing. Keep the fluent ReturnValue[*] rows and the
string and array outputs. Rework the fixture cases that sank an int to
go through getString, and add two no-flow cases for getInt32.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants