Skip to content

Commit 6864e3a

Browse files
mbaludambaluda
committed
STR30-C: use new DataFlow module
1 parent 1274407 commit 6864e3a

2 files changed

Lines changed: 43 additions & 114 deletions

File tree

‎c/cert/src/rules/STR30-C/DoNotAttemptToModifyStringLiterals.ql‎

Lines changed: 43 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -19,141 +19,85 @@
1919
import cpp
2020
import codingstandards.c.cert
2121
import semmle.code.cpp.security.BufferWrite
22-
import semmle.code.cpp.dataflow.DataFlow
23-
24-
/**
25-
* Class that includes into `BufferWrite` functions that will modify their
26-
* first argument. This is an extension of `BufferWrite` which covers the case
27-
* of opaque writes via library functions.
28-
*/
29-
class ModifiesFirstArgFunction extends BufferWrite, FunctionCall {
30-
Expr modifiedExpr;
22+
import semmle.code.cpp.dataflow.new.DataFlow
3123

24+
/** A modeled buffer write through the first argument of a library call. */
25+
private class ModifiesFirstArgFunction extends BufferWrite, FunctionCall {
3226
ModifiesFirstArgFunction() {
33-
getTarget().getName() = ["mkstemp", "memset", "memcpy", "memmove"] and
34-
getArgument(0) = modifiedExpr
27+
getTarget().getName() = ["mkstemp", "memset", "memcpy", "memmove"]
3528
}
3629

3730
override Type getBufferType() { none() }
3831

39-
override Expr getDest() { result = modifiedExpr }
32+
override Expr getDest() { result = getArgument(0) }
4033
}
4134

42-
/**
43-
* Models a dataflow wherein a source is either a implicit or explicit string
44-
* literal that is assigned to a non modifiable type or wherein the string
45-
* literal arises as a argument to a function that may modify its argument.
46-
*/
47-
module ImplicitOrExplicitStringLiteralModifiedConfig implements DataFlow::ConfigSig {
35+
/** Provides dataflow from assigned string literals to writes. */
36+
private module StringLiteralConfig implements DataFlow::ConfigSig {
4837
predicate isSource(DataFlow::Node node) {
49-
// usage through variables
5038
exists(Variable v |
5139
v.getAnAssignedValue() = node.asExpr() and
52-
(
53-
node.asExpr() instanceof ImplicitStringLiteral or
54-
node.asExpr() instanceof StringLiteralOrConstChar
55-
) and
40+
mayBeStringLiteral(node.asExpr()) and
5641
v.getType().getUnderlyingType() instanceof CharPointerType
5742
)
58-
or
59-
// direct usage of string literals as function parameters
60-
exists(BufferWrite bw |
61-
bw.getDest() = node.asExpr() and
62-
(
63-
node.asExpr() instanceof ImplicitStringLiteral or
64-
node.asExpr() instanceof StringLiteralOrConstChar
65-
)
66-
)
6743
}
6844

6945
predicate isSink(DataFlow::Node node) {
70-
// it's either a buffer write of some kind that we
71-
// know about
72-
exists(BufferWrite bw | bw.getDest() = node.asExpr())
46+
node.asExpr() = any(BufferWrite bw).getDest()
7347
or
74-
// or it is a direct assignment of some kind - including reassignment of the pointer
75-
exists(AssignExpr aexp | aexp.getLValue().(ArrayExpr).getArrayBase() = node.asExpr())
48+
node.asExpr() = any(AssignExpr a).getLValue().(ArrayExpr).getArrayBase()
7649
or
77-
exists(AssignExpr aexp | aexp.getLValue().(PointerDereferenceExpr).getOperand() = node.asExpr())
50+
node.asExpr() = any(AssignExpr a).getLValue().(PointerDereferenceExpr).getOperand()
7851
}
7952
}
8053

81-
module ImplicitOrExplicitStringLiteralModifiedFlow =
82-
DataFlow::Global<ImplicitOrExplicitStringLiteralModifiedConfig>;
54+
/** Provides dataflow from possible string literals to writes. */
55+
private module StringLiteralFlow {
56+
private module Global = DataFlow::Global<StringLiteralConfig>;
8357

84-
class MaybeReturnsStringLiteralFunctionCall extends FunctionCall {
85-
MaybeReturnsStringLiteralFunctionCall() {
86-
getTarget().getName() in [
87-
"strpbrk", "strchr", "strrchr", "strstr", "wcspbrk", "wcschr", "wcsrchr", "wcsstr",
88-
"memchr", "wmemchr"
89-
]
58+
/** Holds if `source` may point to a string literal that is written at `sink`. */
59+
predicate flow(Expr source, Expr sink) {
60+
// Report the pointer operand rather than a dereference represented by the same dataflow node.
61+
not sink instanceof PointerDereferenceExpr and
62+
(
63+
Global::flow(DataFlow::exprNode(source), DataFlow::exprNode(sink))
64+
or
65+
source = sink and
66+
mayBeStringLiteral(sink) and
67+
sink = any(BufferWrite bw).getDest()
68+
)
9069
}
9170
}
9271

93-
class ImplicitStringLiteral extends Expr {
72+
/** A call that may return a pointer into a possible string literal. */
73+
private class ImplicitStringLiteral extends FunctionCall {
9474
ImplicitStringLiteral() {
95-
exists(MaybeReturnsStringLiteralFunctionCall fc, Variable e |
96-
e.getAnAssignedValue() = fc and
97-
this = fc and
98-
// additionally, we require that the first argument is either an explicit
99-
// or implicit string literal
100-
(
101-
// directly a string literal
102-
fc.getArgument(0) instanceof StringLiteralOrConstChar
103-
or
104-
// a string literal flows into it
105-
exists(StringLiteralOrConstChar sl |
106-
DataFlow::localFlow(DataFlow::exprNode(sl), DataFlow::exprNode(fc.getArgument(0)))
107-
)
108-
or
109-
// or a base flows into it
110-
exists(ImplicitStringLiteralBase base |
111-
DataFlow::localFlow(DataFlow::exprNode(base), DataFlow::exprNode(fc.getArgument(0)))
112-
)
113-
)
75+
getTarget().getName() in [
76+
"strpbrk", "strchr", "strrchr", "strstr", "wcspbrk", "wcschr", "wcsrchr", "wcsstr",
77+
"memchr", "wmemchr"
78+
] and
79+
exists(Variable v | v.getAnAssignedValue() = this) and
80+
exists(Expr source |
81+
mayBeStringLiteral(source) and DataFlow::localExprFlow(source, getArgument(0))
11482
)
11583
}
11684
}
11785

118-
class StringLiteralOrConstChar extends Expr {
119-
StringLiteralOrConstChar() {
120-
this instanceof StringLiteral
121-
or
122-
getUnspecifiedType() instanceof CharPointerType and
123-
getType().(PointerType).getBaseType().isConst()
124-
}
125-
}
126-
127-
/**
128-
* Since it is possible to produce an implicit literal by either
129-
* an explicit literal being passed to one of these functions this
130-
* class exists to establish the "base" type, that is an explicit
131-
* string literal passed or flowing into the first argument. The other
132-
* Implicit string literal class will then check to see if it is inductively
133-
* an implicit string literal.
134-
*/
135-
class ImplicitStringLiteralBase extends Expr {
136-
ImplicitStringLiteralBase() {
137-
exists(MaybeReturnsStringLiteralFunctionCall fc, Variable e |
138-
e.getAnAssignedValue() = fc and
139-
this = fc and
140-
// it either directly gets a string literal or one via flow
141-
(
142-
fc.getArgument(0) instanceof StringLiteralOrConstChar or
143-
exists(StringLiteralOrConstChar sl |
144-
DataFlow::localFlow(DataFlow::exprNode(sl), DataFlow::exprNode(fc.getArgument(0)))
145-
)
146-
)
147-
)
148-
}
86+
/** Holds if `e` may point to a string literal. */
87+
private predicate mayBeStringLiteral(Expr e) {
88+
e instanceof StringLiteral
89+
or
90+
e.getUnspecifiedType() instanceof CharPointerType and
91+
e.getType().(PointerType).getBaseType().isConst()
92+
or
93+
e instanceof ImplicitStringLiteral
14994
}
15095

15196
from Expr literal, Expr literalWrite
15297
where
15398
not isExcluded(literal, Strings1Package::doNotAttemptToModifyStringLiteralsQuery()) and
15499
not isExcluded(literalWrite, Strings1Package::doNotAttemptToModifyStringLiteralsQuery()) and
155-
ImplicitOrExplicitStringLiteralModifiedFlow::flow(DataFlow::exprNode(literal),
156-
DataFlow::exprNode(literalWrite))
100+
StringLiteralFlow::flow(literal, literalWrite)
157101
select literalWrite,
158102
"This operation may write to a string that may be a string literal that was $@.", literal,
159103
"created here"

‎c/cert/test/rules/STR30-C/DoNotAttemptToModifyStringLiterals.expected‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,3 @@
1-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:47,65-73)
2-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:48,22-30)
3-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:69,20-28)
4-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:82,3-11)
5-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:106,11-19)
6-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:106,31-39)
7-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:106,55-63)
8-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:111,11-19)
9-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:111,31-39)
10-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:111,57-65)
11-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:144,11-19)
12-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:144,31-39)
13-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:144,55-63)
14-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:155,53-61)
15-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:156,5-13)
161
| test.c:7:3:7:3 | a | This operation may write to a string that may be a string literal that was $@. | test.c:6:13:6:20 | codeql | created here |
172
| test.c:30:3:30:3 | a | This operation may write to a string that may be a string literal that was $@. | test.c:29:13:29:18 | call to strchr | created here |
183
| test.c:36:3:36:3 | b | This operation may write to a string that may be a string literal that was $@. | test.c:35:13:35:18 | call to strchr | created here |

0 commit comments

Comments
 (0)