Skip to content

Commit 61597f5

Browse files
committed
C++: This commit does two things:
1. It fixes a logic error in the cannotContainString predicate. 2. It reverts the changes to the `isSource` predicate that required the external function to be within the source root. The change to `isSource` was meant to fix the a performance problem that occurred because of the logic error in the cannotContainString predicate. However, now that the logic error is fixed this is no longer necessary 🎉
1 parent ab6e2f9 commit 61597f5

File tree

1 file changed

+7
-11
lines changed

1 file changed

+7
-11
lines changed

cpp/ql/src/Likely Bugs/Format/NonConstantFormat.ql

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ class UncalledFunction extends Function {
3737
}
3838
}
3939

40+
/** The `unsigned short` type. */
41+
class UnsignedShort extends ShortType {
42+
UnsignedShort() { this.isUnsigned() }
43+
}
44+
4045
/**
4146
* Holds if `t` cannot refer to a string. That is, it's a built-in
4247
* or arithmetic type that is not a "`char` like" type.
@@ -51,7 +56,7 @@ predicate cannotContainString(Type t) {
5156
not unspecified instanceof Char16Type and
5257
not unspecified instanceof Char32Type and
5358
// C often defines `wchar_t` as `unsigned short`
54-
unspecified = any(ShortType short | not short.isUnsigned())
59+
not unspecified instanceof UnsignedShort
5560
|
5661
unspecified instanceof ArithmeticType or
5762
unspecified instanceof BuiltInType
@@ -63,14 +68,6 @@ predicate dataFlowOrTaintFlowFunction(Function func, FunctionOutput output) {
6368
func.(TaintFunction).hasTaintFlow(_, output)
6469
}
6570

66-
/** Holds if `func` is declared inside the source root. */
67-
predicate isInsideSourceRoot(Function func) {
68-
exists(File f |
69-
f = func.getFile() and
70-
exists(f.getRelativePath())
71-
)
72-
}
73-
7471
/**
7572
* Holds if `node` is a non-constant source of data flow for non-const format string detection.
7673
* This is defined as either:
@@ -119,8 +116,7 @@ predicate isNonConst(DataFlow::Node node) {
119116
// The function's output must also not be const to be considered a non-const source
120117
exists(Function func, CallInstruction call |
121118
not func.hasDefinition() and
122-
func = call.getStaticCallTarget() and
123-
isInsideSourceRoot(func)
119+
func = call.getStaticCallTarget()
124120
|
125121
// Case 1: It's a known dataflow or taintflow function with flow to the return value
126122
call.getUnconvertedResultExpression() = node.asIndirectExpr() and

0 commit comments

Comments
 (0)