Skip to content

Commit 86404af

Browse files
authored
Merge pull request #4270 from MathiasVP/mathiasvp/single-field-flow-fix-cwe190test
C++: Use underlying type when checking whether a type is a single-field struct.
2 parents 4b423fe + 3005f25 commit 86404af

File tree

10 files changed

+36
-3
lines changed

10 files changed

+36
-3
lines changed

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,7 @@ pragma[noinline]
557557
private predicate getFieldSizeOfClass(Class c, Type type, int size) {
558558
exists(Field f |
559559
f.getDeclaringType() = c and
560-
f.getType() = type and
560+
f.getUnderlyingType() = type and
561561
type.getSize() = size
562562
)
563563
}
@@ -601,7 +601,7 @@ private predicate simpleOperandLocalFlowStep(Instruction iFrom, Operand opTo) {
601601
exists(LoadInstruction load |
602602
load.getSourceValueOperand() = opTo and
603603
opTo.getAnyDef() = iFrom and
604-
isSingleFieldClass(iFrom.getResultType(), opTo.getType())
604+
isSingleFieldClass(iFrom.getResultType(), opTo.getType().getUnderlyingType())
605605
)
606606
}
607607

cpp/ql/test/library-tests/dataflow/fields/dataflow-ir-consistency.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ localCallNodes
1717
postIsNotPre
1818
postHasUniquePre
1919
| simple.cpp:65:5:65:22 | Store | PostUpdateNode should have one pre-update node but has 0. |
20+
| simple.cpp:92:5:92:22 | Store | PostUpdateNode should have one pre-update node but has 0. |
2021
uniquePostUpdate
2122
postIsInSameCallable
2223
reverseRead

cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,9 @@ edges
167167
| simple.cpp:83:9:83:28 | Store | simple.cpp:83:9:83:28 | Chi [f1] |
168168
| simple.cpp:83:17:83:26 | call to user_input | simple.cpp:83:9:83:28 | Store |
169169
| simple.cpp:84:14:84:20 | Argument -1 indirection [f1] | simple.cpp:84:14:84:20 | call to getf2f1 |
170+
| simple.cpp:92:5:92:22 | Store [i] | simple.cpp:93:20:93:20 | Store [i] |
171+
| simple.cpp:92:11:92:20 | call to user_input | simple.cpp:92:5:92:22 | Store [i] |
172+
| simple.cpp:93:20:93:20 | Store [i] | simple.cpp:94:13:94:13 | i |
170173
| struct_init.c:14:24:14:25 | *ab [a] | struct_init.c:15:12:15:12 | a |
171174
| struct_init.c:20:20:20:29 | Chi [a] | struct_init.c:24:10:24:12 | Argument 0 indirection [a] |
172175
| struct_init.c:20:20:20:29 | Store | struct_init.c:20:20:20:29 | Chi [a] |
@@ -376,6 +379,10 @@ nodes
376379
| simple.cpp:83:17:83:26 | call to user_input | semmle.label | call to user_input |
377380
| simple.cpp:84:14:84:20 | Argument -1 indirection [f1] | semmle.label | Argument -1 indirection [f1] |
378381
| simple.cpp:84:14:84:20 | call to getf2f1 | semmle.label | call to getf2f1 |
382+
| simple.cpp:92:5:92:22 | Store [i] | semmle.label | Store [i] |
383+
| simple.cpp:92:11:92:20 | call to user_input | semmle.label | call to user_input |
384+
| simple.cpp:93:20:93:20 | Store [i] | semmle.label | Store [i] |
385+
| simple.cpp:94:13:94:13 | i | semmle.label | i |
379386
| struct_init.c:14:24:14:25 | *ab [a] | semmle.label | *ab [a] |
380387
| struct_init.c:15:12:15:12 | a | semmle.label | a |
381388
| struct_init.c:20:20:20:29 | Chi [a] | semmle.label | Chi [a] |
@@ -435,6 +442,7 @@ nodes
435442
| simple.cpp:29:12:29:12 | call to b | simple.cpp:42:12:42:21 | call to user_input | simple.cpp:29:12:29:12 | call to b | call to b flows from $@ | simple.cpp:42:12:42:21 | call to user_input | call to user_input |
436443
| simple.cpp:67:13:67:13 | i | simple.cpp:65:11:65:20 | call to user_input | simple.cpp:67:13:67:13 | i | i flows from $@ | simple.cpp:65:11:65:20 | call to user_input | call to user_input |
437444
| simple.cpp:84:14:84:20 | call to getf2f1 | simple.cpp:83:17:83:26 | call to user_input | simple.cpp:84:14:84:20 | call to getf2f1 | call to getf2f1 flows from $@ | simple.cpp:83:17:83:26 | call to user_input | call to user_input |
445+
| simple.cpp:94:13:94:13 | i | simple.cpp:92:11:92:20 | call to user_input | simple.cpp:94:13:94:13 | i | i flows from $@ | simple.cpp:92:11:92:20 | call to user_input | call to user_input |
438446
| struct_init.c:15:12:15:12 | a | struct_init.c:20:20:20:29 | call to user_input | struct_init.c:15:12:15:12 | a | a flows from $@ | struct_init.c:20:20:20:29 | call to user_input | call to user_input |
439447
| struct_init.c:15:12:15:12 | a | struct_init.c:27:7:27:16 | call to user_input | struct_init.c:15:12:15:12 | a | a flows from $@ | struct_init.c:27:7:27:16 | call to user_input | call to user_input |
440448
| struct_init.c:22:11:22:11 | a | struct_init.c:20:20:20:29 | call to user_input | struct_init.c:22:11:22:11 | a | a flows from $@ | struct_init.c:20:20:20:29 | call to user_input | call to user_input |

cpp/ql/test/library-tests/dataflow/fields/partial-definition-diff.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,7 @@
386386
| simple.cpp:83:9:83:10 | this | AST only |
387387
| simple.cpp:83:12:83:13 | f1 | AST only |
388388
| simple.cpp:84:14:84:20 | this | AST only |
389+
| simple.cpp:92:7:92:7 | i | AST only |
389390
| struct_init.c:15:8:15:9 | ab | AST only |
390391
| struct_init.c:15:12:15:12 | a | AST only |
391392
| struct_init.c:16:8:16:9 | ab | AST only |

cpp/ql/test/library-tests/dataflow/fields/partial-definition-ir.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,4 @@
4444
| simple.cpp:21:24:21:25 | this |
4545
| simple.cpp:65:5:65:5 | a |
4646
| simple.cpp:83:9:83:10 | f2 |
47+
| simple.cpp:92:5:92:5 | a |

cpp/ql/test/library-tests/dataflow/fields/partial-definition.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,8 @@
432432
| simple.cpp:83:9:83:10 | this |
433433
| simple.cpp:83:12:83:13 | f1 |
434434
| simple.cpp:84:14:84:20 | this |
435+
| simple.cpp:92:5:92:5 | a |
436+
| simple.cpp:92:7:92:7 | i |
435437
| struct_init.c:15:8:15:9 | ab |
436438
| struct_init.c:15:12:15:12 | a |
437439
| struct_init.c:16:8:16:9 | ab |

cpp/ql/test/library-tests/dataflow/fields/path-flow.expected

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,10 @@ edges
391391
| simple.cpp:83:9:83:28 | ... = ... | simple.cpp:83:9:83:10 | f2 [post update] [f1] |
392392
| simple.cpp:83:17:83:26 | call to user_input | simple.cpp:83:9:83:28 | ... = ... |
393393
| simple.cpp:84:14:84:20 | this [f2, f1] | simple.cpp:84:14:84:20 | call to getf2f1 |
394+
| simple.cpp:92:5:92:5 | a [post update] [i] | simple.cpp:94:10:94:11 | a2 [i] |
395+
| simple.cpp:92:5:92:22 | ... = ... | simple.cpp:92:5:92:5 | a [post update] [i] |
396+
| simple.cpp:92:11:92:20 | call to user_input | simple.cpp:92:5:92:22 | ... = ... |
397+
| simple.cpp:94:10:94:11 | a2 [i] | simple.cpp:94:13:94:13 | i |
394398
| struct_init.c:14:24:14:25 | ab [a] | struct_init.c:15:8:15:9 | ab [a] |
395399
| struct_init.c:15:8:15:9 | ab [a] | struct_init.c:15:12:15:12 | a |
396400
| struct_init.c:20:17:20:36 | {...} [a] | struct_init.c:22:8:22:9 | ab [a] |
@@ -856,6 +860,11 @@ nodes
856860
| simple.cpp:83:17:83:26 | call to user_input | semmle.label | call to user_input |
857861
| simple.cpp:84:14:84:20 | call to getf2f1 | semmle.label | call to getf2f1 |
858862
| simple.cpp:84:14:84:20 | this [f2, f1] | semmle.label | this [f2, f1] |
863+
| simple.cpp:92:5:92:5 | a [post update] [i] | semmle.label | a [post update] [i] |
864+
| simple.cpp:92:5:92:22 | ... = ... | semmle.label | ... = ... |
865+
| simple.cpp:92:11:92:20 | call to user_input | semmle.label | call to user_input |
866+
| simple.cpp:94:10:94:11 | a2 [i] | semmle.label | a2 [i] |
867+
| simple.cpp:94:13:94:13 | i | semmle.label | i |
859868
| struct_init.c:14:24:14:25 | ab [a] | semmle.label | ab [a] |
860869
| struct_init.c:15:8:15:9 | ab [a] | semmle.label | ab [a] |
861870
| struct_init.c:15:12:15:12 | a | semmle.label | a |
@@ -967,6 +976,7 @@ nodes
967976
| simple.cpp:29:12:29:12 | call to b | simple.cpp:42:12:42:21 | call to user_input | simple.cpp:29:12:29:12 | call to b | call to b flows from $@ | simple.cpp:42:12:42:21 | call to user_input | call to user_input |
968977
| simple.cpp:67:13:67:13 | i | simple.cpp:65:11:65:20 | call to user_input | simple.cpp:67:13:67:13 | i | i flows from $@ | simple.cpp:65:11:65:20 | call to user_input | call to user_input |
969978
| simple.cpp:84:14:84:20 | call to getf2f1 | simple.cpp:83:17:83:26 | call to user_input | simple.cpp:84:14:84:20 | call to getf2f1 | call to getf2f1 flows from $@ | simple.cpp:83:17:83:26 | call to user_input | call to user_input |
979+
| simple.cpp:94:13:94:13 | i | simple.cpp:92:11:92:20 | call to user_input | simple.cpp:94:13:94:13 | i | i flows from $@ | simple.cpp:92:11:92:20 | call to user_input | call to user_input |
970980
| struct_init.c:15:12:15:12 | a | struct_init.c:20:20:20:29 | call to user_input | struct_init.c:15:12:15:12 | a | a flows from $@ | struct_init.c:20:20:20:29 | call to user_input | call to user_input |
971981
| struct_init.c:15:12:15:12 | a | struct_init.c:27:7:27:16 | call to user_input | struct_init.c:15:12:15:12 | a | a flows from $@ | struct_init.c:27:7:27:16 | call to user_input | call to user_input |
972982
| struct_init.c:15:12:15:12 | a | struct_init.c:40:20:40:29 | call to user_input | struct_init.c:15:12:15:12 | a | a flows from $@ | struct_init.c:40:20:40:29 | call to user_input | call to user_input |

cpp/ql/test/library-tests/dataflow/fields/simple.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,4 +85,13 @@ struct C2
8585
}
8686
};
8787

88+
typedef A A_typedef;
89+
90+
void single_field_test_typedef(A_typedef a)
91+
{
92+
a.i = user_input();
93+
A_typedef a2 = a;
94+
sink(a2.i); //$ast,ir
95+
}
96+
8897
} // namespace Simple

cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/IntegerOverflowTainted.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
| test2.cpp:14:11:14:15 | ... * ... | $@ flows to here and is used in an expression which might overflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
2+
| test2.cpp:15:11:15:19 | ... * ... | $@ flows to here and is used in an expression which might overflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
23
| test2.cpp:16:11:16:21 | ... * ... | $@ flows to here and is used in an expression which might overflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
34
| test2.cpp:17:11:17:22 | ... * ... | $@ flows to here and is used in an expression which might overflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
45
| test3.c:12:31:12:34 | * ... | $@ flows to here and is used in an expression which might overflow negatively. | test3.c:11:15:11:18 | argv | User-provided value |

cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/test2.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ typedef struct _myStruct {
1212
void test2_sink(s64 v, MyStruct s, MyStruct &s_r, MyStruct *s_p)
1313
{
1414
s64 v1 = v * 2; // bad
15-
s64 v2 = s.val * 2; // bad [NOT DETECTED]
15+
s64 v2 = s.val * 2; // bad
1616
s64 v3 = s_r.val * 2; // bad
1717
s64 v4 = s_p->val * 2; // bad
1818
}

0 commit comments

Comments
 (0)