cpp: model BDE bslx in-stream deserialization taint flow - #22453
Conversation
There was a problem hiding this comment.
Pull request overview
Adds C++ taint-flow models for Bloomberg BDE bslx deserialization.
Changes:
- Models
ByteInStream,GenericInStream, andbdexStreamIn. - 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.
77a4275 to
ad27014
Compare
ad27014 to
c7d50f6
Compare
d889939 to
ccff432
Compare
jketema
left a comment
jketema
left a comment
There was a problem hiding this comment.
Some comments. As before I've tried make some of the text shorter.
geoffw0
left a comment
geoffw0
left a comment
There was a problem hiding this comment.
LGTM, I've left a few comments but I don't think any more changes are needed. We can merge this if CI passes.
| stream.reset(); | ||
| bsl::string out; | ||
| stream.getString(out); | ||
| sink(*out.data()); // $ ir |
There was a problem hiding this comment.
My understanding is that reset() restarts the stream outputting the same data again - hence why there is still flow here. Is that correct?
There was a problem hiding this comment.
The reset() here rewinds the cursor and revalidates the stream while keeping the existing buffer and length. The subsequent reads, therefore, consume the same tainted data, and the flow here is intentional.
| bsl::string out; | ||
| stream.getString(out); | ||
| // Known false positive: this summary does not kill taint from the old buffer. | ||
| sink(*out.data()); // $ SPURIOUS: ir |
There was a problem hiding this comment.
It ought to be possible to fix this spurious result with a Barrier model on the variants of reset with parameters (see here). I don't think this needs to hold up the pull request, but it's something you could experiment with in future.
There was a problem hiding this comment.
Thanks @geoffw0 for the suggestion. I will investigate a barrier model for the reset overloads that replace the buffer in a follow-up. I added a comment noting that this is a known false positive.
e50e4c7 to
4f52935
Compare
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.
51dc80c to
c14358a
Compare
|
Hi @kumarak , can you try to force push less often in future, it makes it harder to keep up as a reviewer. |
Sure, I will avoid force push. I do it to fix the commit history which sometime gets screwed due to merge. |
I'd prefer a somewhat untidy commit history, if you need to make changes after the initial PR. |
geoffw0
left a comment
geoffw0
left a comment
There was a problem hiding this comment.
In any case this is looking great now.
Thanks for the fixes, and thank you for your contribution to CPP analysis! 🎉
Add flow summaries for the BDE byte-stream deserializers in BloombergLP::bslx:
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.