Skip to content

Commit ccff432

Browse files
committed
fix(cpp): address bslx model review feedback
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.
2 parents c7d50f6 + cfc358e commit ccff432

226 files changed

Lines changed: 5636 additions & 3507 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added flow summaries for the Protocol Buffers `google::protobuf::MessageLite` C++ API.

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

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/cpp-all
4+
extensible: summaryModel
5+
data: # namespace, type, subtypes, name, signature, ext, input, output, kind, provenance
6+
# File-descriptor variants (`{Parse,Serialize}*FromFileDescriptor`) are intentionally omitted:
7+
# the descriptor is an `int`, not a data buffer, so there is no buffer argument to model.
8+
9+
# Deserialization
10+
- ["google::protobuf", "MessageLite", True, "ParseFromString", "(string_view)", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
11+
- ["google::protobuf", "MessageLite", True, "ParseFromString", "(const Cord &)", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
12+
- ["google::protobuf", "MessageLite", True, "ParsePartialFromString", "(string_view)", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
13+
- ["google::protobuf", "MessageLite", True, "ParsePartialFromString", "(const Cord &)", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
14+
- ["google::protobuf", "MessageLite", True, "MergeFromString", "(string_view)", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
15+
- ["google::protobuf", "MessageLite", True, "MergeFromString", "(const Cord &)", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
16+
- ["google::protobuf", "MessageLite", True, "MergePartialFromString", "(string_view)", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
17+
- ["google::protobuf", "MessageLite", True, "MergePartialFromString", "(const Cord &)", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
18+
- ["google::protobuf", "MessageLite", True, "ParseFromArray", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
19+
- ["google::protobuf", "MessageLite", True, "ParsePartialFromArray", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
20+
- ["google::protobuf", "MessageLite", True, "ParseFromCord", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
21+
- ["google::protobuf", "MessageLite", True, "ParsePartialFromCord", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
22+
- ["google::protobuf", "MessageLite", True, "MergeFromCord", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
23+
- ["google::protobuf", "MessageLite", True, "MergePartialFromCord", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
24+
- ["google::protobuf", "MessageLite", True, "ParseFromIstream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
25+
- ["google::protobuf", "MessageLite", True, "ParsePartialFromIstream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
26+
- ["google::protobuf", "MessageLite", True, "ParseFromZeroCopyStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
27+
- ["google::protobuf", "MessageLite", True, "ParsePartialFromZeroCopyStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
28+
- ["google::protobuf", "MessageLite", True, "ParseFromBoundedZeroCopyStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
29+
- ["google::protobuf", "MessageLite", True, "ParsePartialFromBoundedZeroCopyStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
30+
- ["google::protobuf", "MessageLite", True, "MergeFromBoundedZeroCopyStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
31+
- ["google::protobuf", "MessageLite", True, "MergePartialFromBoundedZeroCopyStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
32+
- ["google::protobuf", "MessageLite", True, "ParseFromCodedStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
33+
- ["google::protobuf", "MessageLite", True, "ParsePartialFromCodedStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
34+
- ["google::protobuf", "MessageLite", True, "MergeFromCodedStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
35+
- ["google::protobuf", "MessageLite", True, "MergePartialFromCodedStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
36+
37+
# Serialization
38+
- ["google::protobuf", "MessageLite", True, "SerializeToString", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
39+
- ["google::protobuf", "MessageLite", True, "SerializePartialToString", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
40+
- ["google::protobuf", "MessageLite", True, "AppendToString", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
41+
- ["google::protobuf", "MessageLite", True, "AppendPartialToString", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
42+
- ["google::protobuf", "MessageLite", True, "SerializeToArray", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
43+
- ["google::protobuf", "MessageLite", True, "SerializePartialToArray", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
44+
- ["google::protobuf", "MessageLite", True, "SerializeToCord", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
45+
- ["google::protobuf", "MessageLite", True, "SerializePartialToCord", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
46+
- ["google::protobuf", "MessageLite", True, "AppendToCord", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
47+
- ["google::protobuf", "MessageLite", True, "AppendPartialToCord", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
48+
- ["google::protobuf", "MessageLite", True, "SerializeToOstream", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
49+
- ["google::protobuf", "MessageLite", True, "SerializePartialToOstream", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
50+
- ["google::protobuf", "MessageLite", True, "SerializeToZeroCopyStream", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
51+
- ["google::protobuf", "MessageLite", True, "SerializePartialToZeroCopyStream", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
52+
- ["google::protobuf", "MessageLite", True, "SerializeToCodedStream", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
53+
- ["google::protobuf", "MessageLite", True, "SerializePartialToCodedStream", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
54+
55+
# Serialization returning bytes
56+
- ["google::protobuf", "MessageLite", True, "SerializeAsString", "", "", "Argument[-1]", "ReturnValue", "taint", "manual"]
57+
- ["google::protobuf", "MessageLite", True, "SerializePartialAsString", "", "", "Argument[-1]", "ReturnValue", "taint", "manual"]
58+
- ["google::protobuf", "MessageLite", True, "SerializeAsCord", "", "", "Argument[-1]", "ReturnValue", "taint", "manual"]
59+
- ["google::protobuf", "MessageLite", True, "SerializePartialAsCord", "", "", "Argument[-1]", "ReturnValue", "taint", "manual"]

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

Lines changed: 10 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -10,27 +10,10 @@ extensions:
1010
# tainted stream; a stream reset with a clean buffer keeps any earlier taint.
1111
- ["BloombergLP::bslx", "ByteInStream", true, "ByteInStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
1212
- ["BloombergLP::bslx", "ByteInStream", true, "reset", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
13-
# Taint out: the stream (`this`) taints the deserialized output variable/buffer.
14-
- ["BloombergLP::bslx", "ByteInStream", true, "getLength", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
15-
- ["BloombergLP::bslx", "ByteInStream", true, "getVersion", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
16-
- ["BloombergLP::bslx", "ByteInStream", true, "getInt8", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
17-
- ["BloombergLP::bslx", "ByteInStream", true, "getUint8", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
18-
- ["BloombergLP::bslx", "ByteInStream", true, "getInt16", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
19-
- ["BloombergLP::bslx", "ByteInStream", true, "getUint16", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
20-
- ["BloombergLP::bslx", "ByteInStream", true, "getInt24", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
21-
- ["BloombergLP::bslx", "ByteInStream", true, "getUint24", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
22-
- ["BloombergLP::bslx", "ByteInStream", true, "getInt32", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
23-
- ["BloombergLP::bslx", "ByteInStream", true, "getUint32", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
24-
- ["BloombergLP::bslx", "ByteInStream", true, "getInt40", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
25-
- ["BloombergLP::bslx", "ByteInStream", true, "getUint40", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
26-
- ["BloombergLP::bslx", "ByteInStream", true, "getInt48", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
27-
- ["BloombergLP::bslx", "ByteInStream", true, "getUint48", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
28-
- ["BloombergLP::bslx", "ByteInStream", true, "getInt56", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
29-
- ["BloombergLP::bslx", "ByteInStream", true, "getUint56", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
30-
- ["BloombergLP::bslx", "ByteInStream", true, "getInt64", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
31-
- ["BloombergLP::bslx", "ByteInStream", true, "getUint64", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
32-
- ["BloombergLP::bslx", "ByteInStream", true, "getFloat32", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
33-
- ["BloombergLP::bslx", "ByteInStream", true, "getFloat64", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
13+
# Taint out: the stream (`this`) taints the deserialized string/array output buffer.
14+
# Scalar getters (getLength, getVersion, getInt*, getUint*, getFloat*) are deliberately
15+
# not modeled as outputs: most queries sanitize taint through integers, so such rows
16+
# would add nothing. Their fluent `ReturnValue[*]` rows below are still modeled.
3417
- ["BloombergLP::bslx", "ByteInStream", true, "getString", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
3518
- ["BloombergLP::bslx", "ByteInStream", true, "getArrayInt8", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
3619
- ["BloombergLP::bslx", "ByteInStream", true, "getArrayUint8", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
@@ -93,27 +76,10 @@ extensions:
9376
# === bslx::GenericInStream<STREAMBUF>: streambuf-backed in-stream ===
9477
# Taint in: the source buffer/streambuf taints the stream (`this`).
9578
- ["BloombergLP::bslx", "GenericInStream", true, "GenericInStream", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
96-
# Taint out: the stream (`this`) taints the deserialized output variable/buffer.
97-
- ["BloombergLP::bslx", "GenericInStream", true, "getLength", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
98-
- ["BloombergLP::bslx", "GenericInStream", true, "getVersion", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
99-
- ["BloombergLP::bslx", "GenericInStream", true, "getInt8", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
100-
- ["BloombergLP::bslx", "GenericInStream", true, "getUint8", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
101-
- ["BloombergLP::bslx", "GenericInStream", true, "getInt16", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
102-
- ["BloombergLP::bslx", "GenericInStream", true, "getUint16", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
103-
- ["BloombergLP::bslx", "GenericInStream", true, "getInt24", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
104-
- ["BloombergLP::bslx", "GenericInStream", true, "getUint24", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
105-
- ["BloombergLP::bslx", "GenericInStream", true, "getInt32", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
106-
- ["BloombergLP::bslx", "GenericInStream", true, "getUint32", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
107-
- ["BloombergLP::bslx", "GenericInStream", true, "getInt40", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
108-
- ["BloombergLP::bslx", "GenericInStream", true, "getUint40", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
109-
- ["BloombergLP::bslx", "GenericInStream", true, "getInt48", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
110-
- ["BloombergLP::bslx", "GenericInStream", true, "getUint48", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
111-
- ["BloombergLP::bslx", "GenericInStream", true, "getInt56", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
112-
- ["BloombergLP::bslx", "GenericInStream", true, "getUint56", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
113-
- ["BloombergLP::bslx", "GenericInStream", true, "getInt64", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
114-
- ["BloombergLP::bslx", "GenericInStream", true, "getUint64", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
115-
- ["BloombergLP::bslx", "GenericInStream", true, "getFloat32", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
116-
- ["BloombergLP::bslx", "GenericInStream", true, "getFloat64", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
79+
# Taint out: the stream (`this`) taints the deserialized string/array output buffer.
80+
# Scalar getters (getLength, getVersion, getInt*, getUint*, getFloat*) are deliberately
81+
# not modeled as outputs: most queries sanitize taint through integers, so such rows
82+
# would add nothing. Their fluent `ReturnValue[*]` rows below are still modeled.
11783
- ["BloombergLP::bslx", "GenericInStream", true, "getString", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
11884
- ["BloombergLP::bslx", "GenericInStream", true, "getArrayInt8", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
11985
- ["BloombergLP::bslx", "GenericInStream", true, "getArrayUint8", "", "", "Argument[-1]", "Argument[*0]", "taint", "manual"]
@@ -175,5 +141,6 @@ extensions:
175141
- ["BloombergLP::bslx", "GenericInStream", true, "getArrayFloat64", "", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"]
176142
# === bslx::InStreamFunctions::bdexStreamIn: generic BDEX deserialization ===
177143
# Free function template; `InStreamFunctions` is a namespace, so `type` is empty.
178-
- ["BloombergLP::bslx::InStreamFunctions", "", false, "bdexStreamIn", "", "", "Argument[*0]", "Argument[*1]", "taint", "manual"]
144+
# Object outputs are modeled in implementations/Bslx.qll, where their type
145+
# can be checked to exclude scalar outputs.
179146
- ["BloombergLP::bslx::InStreamFunctions", "", false, "bdexStreamIn", "", "", "Argument[*0]", "ReturnValue[*]", "taint", "manual"]

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
private import implementations.Allocation
2+
private import implementations.Bslx
23
private import implementations.Deallocation
34
private import implementations.Fopen
45
private import implementations.Fread
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/** Provides taint models for BDE BDEX deserialization into objects. */
2+
3+
import semmle.code.cpp.models.interfaces.Taint
4+
5+
private class BdexStreamIn extends TaintFunction {
6+
BdexStreamIn() {
7+
this.hasQualifiedName("BloombergLP::bslx::InStreamFunctions", "bdexStreamIn") and
8+
this.getParameter(1).getUnspecifiedType().(ReferenceType).getBaseType().getUnspecifiedType()
9+
instanceof Class
10+
}
11+
12+
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
13+
input.isParameterDeref(0) and
14+
output.isParameterDeref(1)
15+
}
16+
}

cpp/ql/test/library-tests/dataflow/external-models/asio_streams.cpp

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,11 @@
11

22
// --- stub library headers ---
33

4-
namespace std {
5-
typedef unsigned long size_t;
6-
#define SIZE_MAX 0xFFFFFFFF
4+
#include "std_string.h"
75

8-
template <class T> class allocator {
9-
};
10-
11-
template<class charT> struct char_traits {
12-
};
13-
14-
template<class charT, class traits = char_traits<charT>, class Allocator = allocator<charT> >
15-
class basic_string {
16-
public:
17-
basic_string(const charT* s, const Allocator& a = Allocator());
18-
};
19-
20-
typedef basic_string<char> string;
6+
#define SIZE_MAX 0xFFFFFFFF
217

8+
namespace std {
229
class string_view {
2310
public:
2411
string_view(const char* s);

0 commit comments

Comments
 (0)