fix(cpp): address remaining bslx review feedback

Author: kumarak

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

@@ -1,4 +1,4 @@
 ---
 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.
+* Added taint flow summaries for the BDE `bslx` byte-stream deserializers `BloombergLP::bslx::ByteInStream`, `BloombergLP::bslx::GenericInStream`, and `BloombergLP::bslx::InStreamFunctions::bdexStreamIn`.

cpp/ql/lib/ext/bslx.model.yml

@@ -4,16 +4,14 @@ extensions:
       pack: codeql/cpp-all
       extensible: summaryModel
     data: # namespace, type, subtypes, name, signature, ext, input, output, kind, provenance
-      # === bslx::ByteInStream: concrete byte-array in-stream ===
-      # Taint in: the source buffer/streambuf taints the stream (`this`).
-      # Note: MaD taint is additive, so `reset` cannot clear taint from a previously
-      # tainted stream; a stream reset with a clean buffer keeps any earlier taint.
+      # bslx::ByteInStream
       - ["BloombergLP::bslx", "ByteInStream", true, "ByteInStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
-      - ["BloombergLP::bslx", "ByteInStream", true, "reset", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
-      # 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.
+      # Input-taking resets replace the buffer; reset() only rewinds it.
+      - ["BloombergLP::bslx", "ByteInStream", true, "reset", "(const char *,size_t)", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
+      - ["BloombergLP::bslx", "ByteInStream", true, "reset", "(const StringRef &)", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
+      # bslx::ByteInStream getters.
+      # Scalar getters (getLength, getVersion, getInt*, getUint*, getFloat*) are not modeled,
+      # as we are generally not interested in tainted scalar values.
       - ["BloombergLP::bslx", "ByteInStream", true, "getString", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
       - ["BloombergLP::bslx", "ByteInStream", true, "getArrayInt8", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
       - ["BloombergLP::bslx", "ByteInStream", true, "getArrayUint8", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
@@ -33,7 +31,7 @@ extensions:
       - ["BloombergLP::bslx", "ByteInStream", true, "getArrayUint64", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
       - ["BloombergLP::bslx", "ByteInStream", true, "getArrayFloat32", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
       - ["BloombergLP::bslx", "ByteInStream", true, "getArrayFloat64", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
-      # Fluent interface: each get* returns `*this`, keeping the returned stream tainted.
+      # bslx::ByteInStream fluent interface
       - ["BloombergLP::bslx", "ByteInStream", true, "getLength", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"]
       - ["BloombergLP::bslx", "ByteInStream", true, "getVersion", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"]
       - ["BloombergLP::bslx", "ByteInStream", true, "getInt8", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"]
@@ -73,13 +71,11 @@ extensions:
       - ["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 ===
-      # Taint in: the source buffer/streambuf taints the stream (`this`).
+      # bslx::GenericInStream
       - ["BloombergLP::bslx", "GenericInStream", true, "GenericInStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
-      # 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.
       - ["BloombergLP::bslx", "GenericInStream", true, "getString", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
       - ["BloombergLP::bslx", "GenericInStream", true, "getArrayInt8", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
       - ["BloombergLP::bslx", "GenericInStream", true, "getArrayUint8", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
@@ -99,7 +95,7 @@ extensions:
       - ["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.
+      # bslx::GenericInStream fluent interface
       - ["BloombergLP::bslx", "GenericInStream", true, "getLength", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"]
       - ["BloombergLP::bslx", "GenericInStream", true, "getVersion", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"]
       - ["BloombergLP::bslx", "GenericInStream", true, "getInt8", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"]
@@ -139,8 +135,6 @@ extensions:
       - ["BloombergLP::bslx", "GenericInStream", true, "getArrayUint64", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"]
       - ["BloombergLP::bslx", "GenericInStream", true, "getArrayFloat32", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"]
       - ["BloombergLP::bslx", "GenericInStream", true, "getArrayFloat64", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"]
-      # === 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
+      - ["BloombergLP::bslx::InStreamFunctions", "", false, "bdexStreamIn", "", "", "Argument[*0]", "Argument[*1]", "taint", "manual"]
       - ["BloombergLP::bslx::InStreamFunctions", "", false, "bdexStreamIn", "", "", "Argument[*0]", "ReturnValue[*]", "taint", "manual"]

cpp/ql/lib/semmle/code/cpp/models/Models.qll

@@ -1,5 +1,4 @@
 private import implementations.Allocation
-private import implementations.Bslx
 private import implementations.Deallocation
 private import implementations.Fopen
 private import implementations.Fread

cpp/ql/lib/semmle/code/cpp/models/implementations/Bslx.qll

@@ -23,6 +23,10 @@ namespace bsl {
 
 // BDE wraps every package-group namespace in `BloombergLP`; this stub reproduces that.
 namespace BloombergLP {
+namespace bslstl {
+	template <class CHAR_TYPE> class StringRefImp {};
+	typedef StringRefImp<char> StringRef;
+}
 namespace bsls {
 	// `bsls::Types` provides the fixed-width integer aliases used by the stream API.
 	struct Types {
@@ -35,7 +39,9 @@ namespace bslx {
 	public:
 		ByteInStream();
 		ByteInStream(const char *buffer, std::size_t numBytes);
+		void reset();
 		void reset(const char *buffer, std::size_t numBytes);
+		void reset(const bslstl::StringRef &srcData);
 		ByteInStream &getLength(int &variable);
 		ByteInStream &getVersion(int &variable);
 		ByteInStream &getInt8(char &variable);
@@ -211,9 +217,11 @@ void test_bdexStreamIn() {
 	sink(*obj.data()); // $ ir
 }
 
-// --- coverage: call every modeled getter so steps.ql verifies each row is consumed ---
+// --- coverage: verify flow through every modeled getter in a fluent chain ---
 
-void coverage_ByteInStream(BloombergLP::bslx::ByteInStream &stream) {
+void coverage_ByteInStream() {
+	std::string data = std::string(source());
+	BloombergLP::bslx::ByteInStream stream(data.data(), data.size());
 	int i = 0;
 	unsigned int ui = 0;
 	char c = 0;
@@ -235,10 +243,15 @@ void coverage_ByteInStream(BloombergLP::bslx::ByteInStream &stream) {
 	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);
+	bsl::string out;
+	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).getString(out);
+	sink(*out.data()); // $ ir
 }
 
-void coverage_GenericInStream(BloombergLP::bslx::GenericInStream<MyStreamBuf> &stream) {
+void coverage_GenericInStream() {
+	std::string data = std::string(source());
+	MyStreamBuf *sb = (MyStreamBuf *)data.data();
+	BloombergLP::bslx::GenericInStream<MyStreamBuf> stream(sb);
 	int i = 0;
 	unsigned int ui = 0;
 	char c = 0;
@@ -260,17 +273,19 @@ void coverage_GenericInStream(BloombergLP::bslx::GenericInStream<MyStreamBuf> &s
 	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);
+	bsl::string out;
+	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).getString(out);
+	sink(*out.data()); // $ ir
 }
 
-void test_bdexStreamIn_integer_no_flow() {
+void test_bdexStreamIn_integer() {
 	std::string data = std::string(source());
 	BloombergLP::bslx::ByteInStream stream(data.data(), data.size());
 	int x = 0;
 	BloombergLP::bslx::InStreamFunctions::bdexStreamIn(stream, x);
-	sink(x); // no flow: scalar outputs are deliberately not modeled
+	sink(x); // $ ir
 	BloombergLP::bslx::InStreamFunctions::bdexStreamIn(stream, x, 0);
-	sink(x); // no flow: the versioned overload must also exclude scalar outputs
+	sink(x); // $ ir
 	bsl::string out;
 	BloombergLP::bslx::InStreamFunctions::bdexStreamIn(stream, x, 0).getString(out);
 	sink(*out.data()); // $ ir
@@ -297,3 +312,46 @@ void test_bdexStreamIn_object() {
 	BloombergLP::bslx::InStreamFunctions::bdexStreamIn(stream, out, 1);
 	sink(out); // $ ir
 }
+
+namespace StringRefTest {
+	BloombergLP::bslstl::StringRef source();
+}
+
+void test_ByteInStream_reset_stringref() {
+	BloombergLP::bslx::ByteInStream stream;
+	BloombergLP::bslstl::StringRef data = StringRefTest::source();
+	stream.reset(data);
+	bsl::string out;
+	stream.getString(out);
+	sink(*out.data()); // $ ir
+}
+
+void test_ByteInStream_reset_rewind() {
+	std::string data = std::string(source());
+	BloombergLP::bslx::ByteInStream stream(data.data(), data.size());
+	stream.reset();
+	bsl::string out;
+	stream.getString(out);
+	sink(*out.data()); // $ ir
+}
+
+void test_ByteInStream_reset_clean_buffer() {
+	BloombergLP::bslx::ByteInStream stream;
+	const char data[] = "clean";
+	stream.reset(data, *source());
+	stream.reset();
+	bsl::string out;
+	stream.getString(out);
+	sink(*out.data()); // no flow: the tainted length does not taint the buffer
+}
+
+void test_ByteInStream_reset_replaces_buffer() {
+	std::string data = std::string(source());
+	BloombergLP::bslx::ByteInStream stream(data.data(), data.size());
+	const char clean[] = "clean";
+	stream.reset(clean, sizeof(clean));
+	bsl::string out;
+	stream.getString(out);
+	// Known false positive: this summary does not kill taint from the old buffer.
+	sink(*out.data()); // $ SPURIOUS: ir
+}