Skip to content

Commit d84a302

Browse files
committed
fix(cpp): correct protobuf string parse models and expand test coverage
1 parent a7e2584 commit d84a302

5 files changed

Lines changed: 233 additions & 90 deletions

File tree

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,18 @@ extensions:
99
# File-descriptor variants (`{Parse,Serialize}*FromFileDescriptor`) are intentionally omitted:
1010
# the descriptor is an `int`, not a data buffer, so there is no buffer argument to model.
1111

12-
# Deserialization: the encoded input (`Argument[*0]`) taints the message (`this`).
13-
- ["google::protobuf", "MessageLite", True, "ParseFromString", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
14-
- ["google::protobuf", "MessageLite", True, "ParsePartialFromString", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
15-
- ["google::protobuf", "MessageLite", True, "MergeFromString", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
16-
- ["google::protobuf", "MessageLite", True, "MergePartialFromString", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
12+
# Deserialization: the encoded input taints the message (`this`). The `*FromString` methods each
13+
# have a `string_view` overload (the buffer is the by-value argument, so `Argument[0]`) and a
14+
# `const Cord &` overload (the buffer is behind a reference, so `Argument[*0]`). The remaining
15+
# inputs below are pointers or references, so they take `Argument[*0]`.
16+
- ["google::protobuf", "MessageLite", True, "ParseFromString", "(string_view)", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
17+
- ["google::protobuf", "MessageLite", True, "ParseFromString", "(const Cord &)", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
18+
- ["google::protobuf", "MessageLite", True, "ParsePartialFromString", "(string_view)", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
19+
- ["google::protobuf", "MessageLite", True, "ParsePartialFromString", "(const Cord &)", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
20+
- ["google::protobuf", "MessageLite", True, "MergeFromString", "(string_view)", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
21+
- ["google::protobuf", "MessageLite", True, "MergeFromString", "(const Cord &)", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
22+
- ["google::protobuf", "MessageLite", True, "MergePartialFromString", "(string_view)", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
23+
- ["google::protobuf", "MessageLite", True, "MergePartialFromString", "(const Cord &)", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
1724
- ["google::protobuf", "MessageLite", True, "ParseFromArray", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
1825
- ["google::protobuf", "MessageLite", True, "ParsePartialFromArray", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]
1926
- ["google::protobuf", "MessageLite", True, "ParseFromCord", "", "", "Argument[*0]", "Argument[-1]", "taint", "manual"]

cpp/ql/test/library-tests/dataflow/external-models/flow.expected

Lines changed: 34 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,8 @@ models
120120
| 119 | Summary: boost::asio::ip; basic_resolver<InternetProtocol>; false; resolve; (string_view,string_view,flags,error_code &); ; Argument[0..1]; ReturnValue; taint; manual |
121121
| 120 | Summary: boost::asio; ; false; buffer; ; ; Argument[*0]; ReturnValue; taint; manual |
122122
| 121 | Summary: google::protobuf; MessageLite; true; ParseFromArray; ; ; Argument[*0]; Argument[-1]; taint; manual |
123-
| 122 | Summary: google::protobuf; MessageLite; true; ParseFromString; ; ; Argument[*0]; Argument[-1]; taint; manual |
124-
| 123 | Summary: google::protobuf; MessageLite; true; SerializeAsString; ; ; Argument[-1]; ReturnValue; taint; manual |
125-
| 124 | Summary: google::protobuf; MessageLite; true; SerializeToArray; ; ; Argument[-1]; Argument[*0]; taint; manual |
123+
| 122 | Summary: google::protobuf; MessageLite; true; SerializeAsString; ; ; Argument[-1]; ReturnValue; taint; manual |
124+
| 123 | Summary: google::protobuf; MessageLite; true; SerializeToArray; ; ; Argument[-1]; Argument[*0]; taint; manual |
126125
edges
127126
| asio_streams.cpp:129:34:129:44 | read_until output argument | asio_streams.cpp:133:7:133:17 | recv_buffer | provenance | Src:MaD:56 |
128127
| asio_streams.cpp:129:34:129:44 | read_until output argument | asio_streams.cpp:135:29:135:39 | recv_buffer | provenance | Src:MaD:56 Sink:MaD:4 |
@@ -267,26 +266,21 @@ edges
267266
| bdlbb.cpp:94:46:94:48 | *src | bdlbb.cpp:94:37:94:40 | copy output argument | provenance | MaD:100 |
268267
| bdlbb.cpp:96:37:96:39 | copy output argument | bdlbb.cpp:97:7:97:10 | * ... | provenance | |
269268
| bdlbb.cpp:96:42:96:44 | *dst | bdlbb.cpp:96:37:96:39 | copy output argument | provenance | MaD:102 |
270-
| protobuf.cpp:75:33:75:40 | call to source | protobuf.cpp:76:22:76:25 | *data | provenance | TaintFunction |
271-
| protobuf.cpp:76:2:76:4 | ParseFromString output argument | protobuf.cpp:78:2:78:4 | *msg | provenance | |
272-
| protobuf.cpp:76:22:76:25 | *data | protobuf.cpp:76:2:76:4 | ParseFromString output argument | provenance | MaD:122 |
273-
| protobuf.cpp:78:2:78:4 | *msg | protobuf.cpp:78:23:78:25 | SerializeToArray output argument | provenance | MaD:124 |
274-
| protobuf.cpp:78:23:78:25 | SerializeToArray output argument | protobuf.cpp:79:7:79:10 | * ... | provenance | |
275-
| protobuf.cpp:84:33:84:40 | call to source | protobuf.cpp:85:21:85:31 | *call to data | provenance | TaintFunction |
276-
| protobuf.cpp:85:2:85:4 | ParseFromArray output argument | protobuf.cpp:87:2:87:4 | *msg | provenance | |
277-
| protobuf.cpp:85:21:85:31 | *call to data | protobuf.cpp:85:2:85:4 | ParseFromArray output argument | provenance | MaD:121 |
278-
| protobuf.cpp:87:2:87:4 | *msg | protobuf.cpp:87:23:87:25 | SerializeToArray output argument | provenance | MaD:124 |
279-
| protobuf.cpp:87:23:87:25 | SerializeToArray output argument | protobuf.cpp:88:7:88:10 | * ... | provenance | |
280-
| protobuf.cpp:95:33:95:40 | call to source | protobuf.cpp:96:22:96:25 | *data | provenance | TaintFunction |
281-
| protobuf.cpp:96:2:96:4 | ParseFromString output argument | protobuf.cpp:98:23:98:25 | *msg | provenance | |
282-
| protobuf.cpp:96:22:96:25 | *data | protobuf.cpp:96:2:96:4 | ParseFromString output argument | provenance | MaD:122 |
283-
| protobuf.cpp:98:2:98:5 | ParseFromString output argument | protobuf.cpp:100:2:100:5 | *msg2 | provenance | |
284-
| protobuf.cpp:98:23:98:25 | *msg | protobuf.cpp:98:27:98:43 | call to SerializeAsString | provenance | MaD:123 |
285-
| protobuf.cpp:98:23:98:45 | call to SerializeAsString | protobuf.cpp:98:2:98:5 | ParseFromString output argument | provenance | MaD:122 |
286-
| protobuf.cpp:98:27:98:43 | call to SerializeAsString | protobuf.cpp:98:23:98:45 | call to SerializeAsString | provenance | |
287-
| protobuf.cpp:98:27:98:43 | call to SerializeAsString | protobuf.cpp:98:27:98:43 | call to SerializeAsString | provenance | |
288-
| protobuf.cpp:100:2:100:5 | *msg2 | protobuf.cpp:100:24:100:26 | SerializeToArray output argument | provenance | MaD:124 |
289-
| protobuf.cpp:100:24:100:26 | SerializeToArray output argument | protobuf.cpp:101:7:101:10 | * ... | provenance | |
269+
| protobuf.cpp:140:33:140:40 | call to source | protobuf.cpp:141:21:141:31 | *call to data | provenance | TaintFunction |
270+
| protobuf.cpp:141:2:141:4 | ParseFromArray output argument | protobuf.cpp:143:2:143:4 | *msg | provenance | |
271+
| protobuf.cpp:141:21:141:31 | *call to data | protobuf.cpp:141:2:141:4 | ParseFromArray output argument | provenance | MaD:121 |
272+
| protobuf.cpp:143:2:143:4 | *msg | protobuf.cpp:143:23:143:25 | SerializeToArray output argument | provenance | MaD:123 |
273+
| protobuf.cpp:143:23:143:25 | SerializeToArray output argument | protobuf.cpp:144:7:144:10 | * ... | provenance | |
274+
| protobuf.cpp:151:33:151:40 | call to source | protobuf.cpp:152:21:152:31 | *call to data | provenance | TaintFunction |
275+
| protobuf.cpp:152:2:152:4 | ParseFromArray output argument | protobuf.cpp:153:18:153:20 | *msg | provenance | |
276+
| protobuf.cpp:152:21:152:31 | *call to data | protobuf.cpp:152:2:152:4 | ParseFromArray output argument | provenance | MaD:121 |
277+
| protobuf.cpp:153:18:153:20 | *msg | protobuf.cpp:153:22:153:38 | call to SerializeAsString | provenance | MaD:122 |
278+
| protobuf.cpp:153:22:153:38 | call to SerializeAsString | protobuf.cpp:153:22:153:38 | call to SerializeAsString | provenance | |
279+
| protobuf.cpp:153:22:153:38 | call to SerializeAsString | protobuf.cpp:155:22:155:29 | *call to data | provenance | TaintFunction |
280+
| protobuf.cpp:155:2:155:5 | ParseFromArray output argument | protobuf.cpp:157:2:157:5 | *msg2 | provenance | |
281+
| protobuf.cpp:155:22:155:29 | *call to data | protobuf.cpp:155:2:155:5 | ParseFromArray output argument | provenance | MaD:121 |
282+
| protobuf.cpp:157:2:157:5 | *msg2 | protobuf.cpp:157:24:157:26 | SerializeToArray output argument | provenance | MaD:123 |
283+
| protobuf.cpp:157:24:157:26 | SerializeToArray output argument | protobuf.cpp:158:7:158:10 | * ... | provenance | |
290284
| test.cpp:7:47:7:52 | value2 | test.cpp:7:64:7:69 | value2 | provenance | |
291285
| test.cpp:7:64:7:69 | value2 | test.cpp:7:5:7:30 | *ymlStepGenerated_with_body | provenance | |
292286
| test.cpp:10:10:10:18 | call to ymlSource | test.cpp:10:10:10:18 | call to ymlSource | provenance | Src:MaD:48 |
@@ -789,29 +783,23 @@ nodes
789783
| bdlbb.cpp:96:37:96:39 | copy output argument | semmle.label | copy output argument |
790784
| bdlbb.cpp:96:42:96:44 | *dst | semmle.label | *dst |
791785
| bdlbb.cpp:97:7:97:10 | * ... | semmle.label | * ... |
792-
| protobuf.cpp:75:33:75:40 | call to source | semmle.label | call to source |
793-
| protobuf.cpp:76:2:76:4 | ParseFromString output argument | semmle.label | ParseFromString output argument |
794-
| protobuf.cpp:76:22:76:25 | *data | semmle.label | *data |
795-
| protobuf.cpp:78:2:78:4 | *msg | semmle.label | *msg |
796-
| protobuf.cpp:78:23:78:25 | SerializeToArray output argument | semmle.label | SerializeToArray output argument |
797-
| protobuf.cpp:79:7:79:10 | * ... | semmle.label | * ... |
798-
| protobuf.cpp:84:33:84:40 | call to source | semmle.label | call to source |
799-
| protobuf.cpp:85:2:85:4 | ParseFromArray output argument | semmle.label | ParseFromArray output argument |
800-
| protobuf.cpp:85:21:85:31 | *call to data | semmle.label | *call to data |
801-
| protobuf.cpp:87:2:87:4 | *msg | semmle.label | *msg |
802-
| protobuf.cpp:87:23:87:25 | SerializeToArray output argument | semmle.label | SerializeToArray output argument |
803-
| protobuf.cpp:88:7:88:10 | * ... | semmle.label | * ... |
804-
| protobuf.cpp:95:33:95:40 | call to source | semmle.label | call to source |
805-
| protobuf.cpp:96:2:96:4 | ParseFromString output argument | semmle.label | ParseFromString output argument |
806-
| protobuf.cpp:96:22:96:25 | *data | semmle.label | *data |
807-
| protobuf.cpp:98:2:98:5 | ParseFromString output argument | semmle.label | ParseFromString output argument |
808-
| protobuf.cpp:98:23:98:25 | *msg | semmle.label | *msg |
809-
| protobuf.cpp:98:23:98:45 | call to SerializeAsString | semmle.label | call to SerializeAsString |
810-
| protobuf.cpp:98:27:98:43 | call to SerializeAsString | semmle.label | call to SerializeAsString |
811-
| protobuf.cpp:98:27:98:43 | call to SerializeAsString | semmle.label | call to SerializeAsString |
812-
| protobuf.cpp:100:2:100:5 | *msg2 | semmle.label | *msg2 |
813-
| protobuf.cpp:100:24:100:26 | SerializeToArray output argument | semmle.label | SerializeToArray output argument |
814-
| protobuf.cpp:101:7:101:10 | * ... | semmle.label | * ... |
786+
| protobuf.cpp:140:33:140:40 | call to source | semmle.label | call to source |
787+
| protobuf.cpp:141:2:141:4 | ParseFromArray output argument | semmle.label | ParseFromArray output argument |
788+
| protobuf.cpp:141:21:141:31 | *call to data | semmle.label | *call to data |
789+
| protobuf.cpp:143:2:143:4 | *msg | semmle.label | *msg |
790+
| protobuf.cpp:143:23:143:25 | SerializeToArray output argument | semmle.label | SerializeToArray output argument |
791+
| protobuf.cpp:144:7:144:10 | * ... | semmle.label | * ... |
792+
| protobuf.cpp:151:33:151:40 | call to source | semmle.label | call to source |
793+
| protobuf.cpp:152:2:152:4 | ParseFromArray output argument | semmle.label | ParseFromArray output argument |
794+
| protobuf.cpp:152:21:152:31 | *call to data | semmle.label | *call to data |
795+
| protobuf.cpp:153:18:153:20 | *msg | semmle.label | *msg |
796+
| protobuf.cpp:153:22:153:38 | call to SerializeAsString | semmle.label | call to SerializeAsString |
797+
| protobuf.cpp:153:22:153:38 | call to SerializeAsString | semmle.label | call to SerializeAsString |
798+
| protobuf.cpp:155:2:155:5 | ParseFromArray output argument | semmle.label | ParseFromArray output argument |
799+
| protobuf.cpp:155:22:155:29 | *call to data | semmle.label | *call to data |
800+
| protobuf.cpp:157:2:157:5 | *msg2 | semmle.label | *msg2 |
801+
| protobuf.cpp:157:24:157:26 | SerializeToArray output argument | semmle.label | SerializeToArray output argument |
802+
| protobuf.cpp:158:7:158:10 | * ... | semmle.label | * ... |
815803
| test.cpp:7:5:7:30 | *ymlStepGenerated_with_body | semmle.label | *ymlStepGenerated_with_body |
816804
| test.cpp:7:47:7:52 | value2 | semmle.label | value2 |
817805
| test.cpp:7:64:7:69 | value2 | semmle.label | value2 |

0 commit comments

Comments
 (0)