Skip to content

STR30-C: use new DataFlow module #1203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mbaluda wants to merge 2 commits into
base: main
Choose a base branch
from
+43 −116
Open

STR30-C: use new DataFlow module #1203

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6864e3a
STR30-C: use new DataFlow module
mbaluda Sep 24, 2026
91027c9
fix formatting
mbaluda Sep 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 43 additions & 101 deletions c/cert/src/rules/STR30-C/DoNotAttemptToModifyStringLiterals.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,141 +19,83 @@
import cpp
import codingstandards.c.cert
import semmle.code.cpp.security.BufferWrite
import semmle.code.cpp.dataflow.DataFlow
import semmle.code.cpp.dataflow.new.DataFlow

/**
* Class that includes into `BufferWrite` functions that will modify their
* first argument. This is an extension of `BufferWrite` which covers the case
* of opaque writes via library functions.
*/
class ModifiesFirstArgFunction extends BufferWrite, FunctionCall {
Expr modifiedExpr;

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

override Type getBufferType() { none() }

override Expr getDest() { result = modifiedExpr }
override Expr getDest() { result = getArgument(0) }
}

/**
* Models a dataflow wherein a source is either a implicit or explicit string
* literal that is assigned to a non modifiable type or wherein the string
* literal arises as a argument to a function that may modify its argument.
*/
module ImplicitOrExplicitStringLiteralModifiedConfig implements DataFlow::ConfigSig {
/** Provides dataflow from assigned string literals to writes. */
private module StringLiteralConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node node) {
// usage through variables
exists(Variable v |
v.getAnAssignedValue() = node.asExpr() and
(
node.asExpr() instanceof ImplicitStringLiteral or
node.asExpr() instanceof StringLiteralOrConstChar
) and
mayBeStringLiteral(node.asExpr()) and
v.getType().getUnderlyingType() instanceof CharPointerType
)
or
// direct usage of string literals as function parameters
exists(BufferWrite bw |
bw.getDest() = node.asExpr() and
(
node.asExpr() instanceof ImplicitStringLiteral or
node.asExpr() instanceof StringLiteralOrConstChar
)
)
}

predicate isSink(DataFlow::Node node) {
// it's either a buffer write of some kind that we
// know about
exists(BufferWrite bw | bw.getDest() = node.asExpr())
node.asExpr() = any(BufferWrite bw).getDest()
or
// or it is a direct assignment of some kind - including reassignment of the pointer
exists(AssignExpr aexp | aexp.getLValue().(ArrayExpr).getArrayBase() = node.asExpr())
node.asExpr() = any(AssignExpr a).getLValue().(ArrayExpr).getArrayBase()
or
exists(AssignExpr aexp | aexp.getLValue().(PointerDereferenceExpr).getOperand() = node.asExpr())
node.asExpr() = any(AssignExpr a).getLValue().(PointerDereferenceExpr).getOperand()
}
}

module ImplicitOrExplicitStringLiteralModifiedFlow =
DataFlow::Global<ImplicitOrExplicitStringLiteralModifiedConfig>;
/** Provides dataflow from possible string literals to writes. */
private module StringLiteralFlow {
private module Global = DataFlow::Global<StringLiteralConfig>;

class MaybeReturnsStringLiteralFunctionCall extends FunctionCall {
MaybeReturnsStringLiteralFunctionCall() {
getTarget().getName() in [
"strpbrk", "strchr", "strrchr", "strstr", "wcspbrk", "wcschr", "wcsrchr", "wcsstr",
"memchr", "wmemchr"
]
/** Holds if `source` may point to a string literal that is written at `sink`. */
predicate flow(Expr source, Expr sink) {
// Report the pointer operand rather than a dereference represented by the same dataflow node.
not sink instanceof PointerDereferenceExpr and
(
Global::flow(DataFlow::exprNode(source), DataFlow::exprNode(sink))
or
source = sink and
mayBeStringLiteral(sink) and
sink = any(BufferWrite bw).getDest()
)
}
}

class ImplicitStringLiteral extends Expr {
/** A call that may return a pointer into a possible string literal. */
private class ImplicitStringLiteral extends FunctionCall {
ImplicitStringLiteral() {
exists(MaybeReturnsStringLiteralFunctionCall fc, Variable e |
e.getAnAssignedValue() = fc and
this = fc and
// additionally, we require that the first argument is either an explicit
// or implicit string literal
(
// directly a string literal
fc.getArgument(0) instanceof StringLiteralOrConstChar
or
// a string literal flows into it
exists(StringLiteralOrConstChar sl |
DataFlow::localFlow(DataFlow::exprNode(sl), DataFlow::exprNode(fc.getArgument(0)))
)
or
// or a base flows into it
exists(ImplicitStringLiteralBase base |
DataFlow::localFlow(DataFlow::exprNode(base), DataFlow::exprNode(fc.getArgument(0)))
)
)
getTarget().getName() in [
"strpbrk", "strchr", "strrchr", "strstr", "wcspbrk", "wcschr", "wcsrchr", "wcsstr",
"memchr", "wmemchr"
] and
exists(Variable v | v.getAnAssignedValue() = this) and
exists(Expr source |
mayBeStringLiteral(source) and DataFlow::localExprFlow(source, getArgument(0))
)
}
}

class StringLiteralOrConstChar extends Expr {
StringLiteralOrConstChar() {
this instanceof StringLiteral
or
getUnspecifiedType() instanceof CharPointerType and
getType().(PointerType).getBaseType().isConst()
}
}

/**
* Since it is possible to produce an implicit literal by either
* an explicit literal being passed to one of these functions this
* class exists to establish the "base" type, that is an explicit
* string literal passed or flowing into the first argument. The other
* Implicit string literal class will then check to see if it is inductively
* an implicit string literal.
*/
class ImplicitStringLiteralBase extends Expr {
ImplicitStringLiteralBase() {
exists(MaybeReturnsStringLiteralFunctionCall fc, Variable e |
e.getAnAssignedValue() = fc and
this = fc and
// it either directly gets a string literal or one via flow
(
fc.getArgument(0) instanceof StringLiteralOrConstChar or
exists(StringLiteralOrConstChar sl |
DataFlow::localFlow(DataFlow::exprNode(sl), DataFlow::exprNode(fc.getArgument(0)))
)
)
)
}
/** Holds if `e` may point to a string literal. */
private predicate mayBeStringLiteral(Expr e) {
e instanceof StringLiteral
or
e.getUnspecifiedType() instanceof CharPointerType and
e.getType().(PointerType).getBaseType().isConst()
or
e instanceof ImplicitStringLiteral
}

from Expr literal, Expr literalWrite
where
not isExcluded(literal, Strings1Package::doNotAttemptToModifyStringLiteralsQuery()) and
not isExcluded(literalWrite, Strings1Package::doNotAttemptToModifyStringLiteralsQuery()) and
ImplicitOrExplicitStringLiteralModifiedFlow::flow(DataFlow::exprNode(literal),
DataFlow::exprNode(literalWrite))
StringLiteralFlow::flow(literal, literalWrite)
select literalWrite,
"This operation may write to a string that may be a string literal that was $@.", literal,
"created here"
15 changes: 0 additions & 15 deletions c/cert/test/rules/STR30-C/DoNotAttemptToModifyStringLiterals.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,3 @@
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:47,65-73)
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:48,22-30)
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:69,20-28)
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:82,3-11)
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:106,11-19)
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:106,31-39)
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:106,55-63)
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:111,11-19)
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:111,31-39)
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:111,57-65)
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:144,11-19)
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:144,31-39)
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:144,55-63)
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:155,53-61)
WARNING: module 'DataFlow' has been deprecated and may be removed in future (DoNotAttemptToModifyStringLiterals.ql:156,5-13)
| 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 |
| 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 |
| 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 |
Expand Down
Loading