hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-20 10:46:30 +01:00
Code Issues Packages Projects Releases Wiki Activity

[CPP-370] Rewrite of NonConstantFormat.ql using the taint tracking library.

Browse Source
This commit is contained in:
Ziemowit Laski
2019-05-08 12:13:20 -07:00
parent a962cff5df
commit ed67c9fd5a
4 changed files with 82 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files

78
cpp/ql/src/Likely Bugs/Format/NonConstantFormat.ql
View File

@@ -14,7 +14,7 @@
* external/cwe/cwe-134
*/
import semmle.code.cpp.dataflow.DataFlow
import semmle.code.cpp.dataflow.TaintTracking
import semmle.code.cpp.commons.Printf
// For the following `...gettext` functions, we assume that
@@ -42,30 +42,52 @@ predicate whitelistFunction(Function f, int arg) {
(arg = 1 or arg = 2)
}
predicate underscoreMacro(Expr e) {
exists(MacroInvocation mi |
mi.getMacroName() = "_" and
mi.getExpr() = e
predicate whitelisted(FunctionCall fc) {
exists(Function f, int arg | f = fc.getTarget() | whitelistFunction(f, arg))
}
predicate isNonConst(Expr e) {
exists(FunctionCall fc | fc = e.(FunctionCall) |
not whitelisted(fc) and not fc.getTarget().hasDefinition()
)
or
exists(Parameter p | p = e.(VariableAccess).getTarget().(Parameter) |
p.getFunction().getName() = "main" and p.getType() instanceof PointerType
)
or
e instanceof CrementOperation
or
e instanceof AddressOfExpr
or
e instanceof ReferenceToExpr
or
e instanceof AssignPointerAddExpr
or
e instanceof AssignPointerSubExpr
or
e instanceof PointerArithmeticOperation
or
e instanceof FieldAccess
or
e instanceof PointerDereferenceExpr
or
e instanceof AddressOfExpr
or
e instanceof ExprCall
or
e instanceof NewArrayExpr
or
e instanceof AssignExpr
or
exists(Variable v | v = e.(VariableAccess).getTarget() |
v.getType().(ArrayType).getBaseType() instanceof CharType and
exists(AssignExpr ae |
ae.getLValue().(ArrayExpr).getArrayBase().(VariableAccess).getTarget() = v
)
)
}
predicate whitelisted(Expr e) {
exists(FunctionCall fc, int arg | fc = e.(FunctionCall) |
whitelistFunction(fc.getTarget(), arg) and
isConst(fc.getArgument(arg))
)
or
// we let the '_' macro through regardless of what it points at
underscoreMacro(e)
}
predicate isConst(Expr e) {
e instanceof StringLiteral
or
whitelisted(e)
}
class NonConstFlow extends DataFlow::Configuration {
class NonConstFlow extends TaintTracking::Configuration {
NonConstFlow() { this = "NonConstFlow" }
override predicate isSource(DataFlow::Node source) { isNonConst(source.asExpr()) }
@@ -73,18 +95,6 @@ class NonConstFlow extends DataFlow::Configuration {
override predicate isSink(DataFlow::Node sink) {
exists(FormattingFunctionCall fc | sink.asExpr() = fc.getArgument(fc.getFormatParameterIndex()))
}
override predicate isAdditionalFlowStep(DataFlow::Node source, DataFlow::Node sink) {
// an element picked from an array of string literals is a string literal
exists(Variable v, int a |
a = sink.asExpr().(ArrayExpr).getArrayOffset().getValue().toInt() and
v = sink.asExpr().(ArrayExpr).getArrayBase().(VariableAccess).getTarget()
|
// we disallow parameters, since they may be bound to unsafe arguments
// at various call sites.
not v instanceof Parameter and source.asExpr() instanceof StringLiteral
)
}
}
from FormattingFunctionCall call, Expr formatString

23
cpp/ql/test/query-tests/Likely Bugs/Format/NonConstantFormat/NonConstantFormat.c
View File

@@ -27,28 +27,29 @@ extern char *any_random_function(const char *);
#define NULL ((void*)0)
// The `_` macro is treated specially. While it is typically set to
// `gettext`, we allow it to point at any function.
#define _(X) my_gettext(X)
int main(int argc, char **argv) {
if(argc > 1)
printf(argv[1]); // not ok
printf(argv[1]); // NOT OK
else
printf("No argument supplied.\n"); // ok
printf("No argument supplied.\n"); // OK
printf(_("No argument supplied.\n")); // ok
printf(_("No argument supplied.\n")); // NOT OK
printf(dgettext(NULL, "No argument supplied.\n")); // ok
printf(dgettext(NULL, "No argument supplied.\n")); // OK
printf(ngettext("One argument\n", "%d arguments\n", argc-1), argc-1); // ok
printf(ngettext("One argument\n", "%d arguments\n", argc-1), argc-1); // OK
printf(gettext("%d arguments\n"), argc-1); // ok
printf(any_random_function("%d arguments\n"), argc-1); // not ok
printf(gettext("%d arguments\n"), argc-1); // OK
printf(any_random_function("%d arguments\n"), argc-1); // NOT OK
#undef _
printf(_(any_random_function("%d arguments\n")),
argc-1); // not ok
/* The special `..gettext..` functions are allowed arbitrary arguments */
printf(_(any_random_function("%d arguments\n")), // OK
argc-1);
printf(_("%d more arguments\n"), // OK
argc-1);
return 0;
}

36
cpp/ql/test/query-tests/Likely Bugs/Format/NonConstantFormat/NonConstantFormat.expected
View File

@@ -1,19 +1,17 @@
| NonConstantFormat.c:36:3:36:8 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| NonConstantFormat.c:47:2:47:7 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| NonConstantFormat.c:50:2:50:7 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:48:3:48:8 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:54:5:54:10 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:56:5:56:10 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:57:5:57:10 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:58:5:58:10 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:59:5:59:10 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:60:5:60:10 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:61:5:61:10 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:62:5:62:10 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:64:3:64:8 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:67:5:67:10 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:73:5:73:10 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:79:5:79:10 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:85:5:85:10 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:90:5:90:10 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:107:5:107:10 | call to printf | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| NonConstantFormat.c:34:10:34:16 | access to array | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| NonConstantFormat.c:38:9:38:36 | call to my_gettext | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| NonConstantFormat.c:45:9:45:27 | call to any_random_function | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:60:12:60:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:63:12:63:21 | call to const_wash | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:64:12:64:26 | ... + ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:65:12:65:17 | + ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:66:12:66:18 | * ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:67:12:67:18 | & ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:68:12:68:39 | ... + ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:70:10:70:35 | ... + ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:73:12:73:20 | ... + ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:79:12:79:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:85:12:85:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:91:12:91:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:96:12:96:18 | ++ ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:113:12:113:24 | new[] | The format string argument to printf should be constant to prevent security issues and other potential errors. |

12
cpp/ql/test/query-tests/Likely Bugs/Format/NonConstantFormat/test.cpp
View File

@@ -10,6 +10,10 @@ const char *messages[] = {
"%u tasks left\n",
};
const char *simple_func(const char *str) {
return str;
}
const char *choose_message(unsigned int n) {
if (n == 0) {
const char *message = messages[0];
@@ -23,7 +27,7 @@ const char *choose_message(unsigned int n) {
const char *make_message(unsigned int n) {
static char buf[64];
sprintf(buf, "%d tasks left\n", n); // ok
sprintf(buf, "%d tasks left\n", n); // OK
return buf;
}
@@ -42,18 +46,20 @@ const char *const_wash(char *str) {
int main(int argc, char **argv) {
const char *message = messages[2];
printf(simple_func("Hello, World\n")); // OK
printf(choose_message(argc - 1), argc - 1); // OK
printf(messages[1]); // OK
printf(message); // OK
printf(make_message(argc - 1)); // NOT OK
printf(make_message(argc - 1)); // OK
printf("Hello, World\n"); // OK
printf(gettext("Hello, World\n")); // OK
printf(_("Hello, World\n")); // OK
{
char hello[] = "hello, World\n";
hello[0] = 'H';
printf(hello); // NOT OK
printf(_(hello)); // OK
printf(gettext(hello)); // NOT OK
printf(gettext(hello)); // OK
printf(const_wash(hello)); // NOT OK
printf((hello + 1) + 1); // NOT OK
printf(+hello); // NOT OK