hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-22 15:25:18 +02:00
Code Issues Packages Projects Releases Wiki Activity

Updating non-const source logic and associated tests and expected files.

Browse Source
This commit is contained in:
Benjamin Rodes
2024-02-07 13:54:56 -05:00
parent 80bf38d8cc
commit 9fc2405681
4 changed files with 82 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -26,7 +26,11 @@ import semmle.code.cpp.ir.implementation.raw.Instruction
class UncalledFunction extends Function {
UncalledFunction() {
not exists(Call c | c.getTarget() = this) and
not this.(MemberFunction).overrides(_)
// TODO: Need rationale here, added based on suggestion
//but unclear of the scenario being avoided
not this.(MemberFunction).overrides(_) and
// Ignore functions that appear to be function pointers
not exists(FunctionAccess fa | fa.getTarget() = this)
}
}
@@ -34,10 +38,15 @@ class UncalledFunction extends Function {
* Holds if `node` is a non-constant source of data flow.
* This is defined as either:
* 1) a `FlowSource`
* 2) a parameter of an 'uncalled' function (i.e., a function that is not called in the program)
* 2) a parameter of an 'uncalled' function
* 3) an argument to a function with no definition that is not known to define the output through its input
* 4) an out arg of a function with no definition that is not known to define the output through its input
*
* With exception to `FlowSource` all non-const values have a type that is not const
* (declared without a `const` specifier)
* ASSUMPTION: any const values are assumed to be static if their assignment is not seen (
* i.e., assuming users did not get non-const data and cast into a const
*
* The latter two cases address identifying standard string manipulation libraries as input sources
* e.g., strcpy, but it will identify unknown function calls as possible non-constant source
* since it cannot be determined if the out argument or return is constant.
@@ -45,32 +54,66 @@ class UncalledFunction extends Function {
predicate isNonConst(DataFlow::Node node) {
node instanceof FlowSource
or
exists(UncalledFunction f | f.getAParameter() = node.asParameter())
// Parameters of uncalled functions that aren't const
exists(UncalledFunction f, Parameter p, Type t |
not t.isConst() and
f.getAParameter() = p and
p = node.asParameter() and
not f.getType().getUnderlyingType().isConst() and
(
// const char* means (const char)*, so the pointer is not const, the pointed to value is
// Grab the base type if a pointer, as this is the type we will check for const-ness
if p.getType().getUnderlyingType() instanceof PointerType
then t = p.getType().getUnderlyingType().(PointerType).getBaseType()
else t = p.getType().getUnderlyingType()
)
)
or
// Consider as an input any out arg of a function or a function's return where the function is not:
// 1. a function with a known dataflow or taintflow from input to output and the `node` is the output
// 2. a function where there is a known definition
// i.e., functions that with unknown bodies and are not known to define the output through its input
// are considered as possible non-const sources
(
node instanceof DataFlow::DefinitionByReferenceNode or
node.asIndirectExpr() instanceof Call
) and
not exists(Function func, FunctionInput input, FunctionOutput output, CallInstruction call |
// NOTE: we must include dataflow and taintflow. e.g., including only dataflow we will find sprintf
// variant function's output are now possible non-const sources
(
func.(DataFlowFunction).hasDataFlow(input, output) or
func.(TaintFunction).hasTaintFlow(input, output)
// The function's output must also not be const to be considered a non-const source
exists(Type t |
not t.isConst() and
exists(Call c |
exists(Expr arg | c.getAnArgument() = arg |
arg = node.asDefiningArgument() and
(
// const char* means (const char)*, so the pointer is not const, the pointed to value is
// Grab the base type if a pointer, as this is the type we will check for const-ness
if arg.getType().getUnderlyingType() instanceof PointerType
then t = arg.getType().getUnderlyingType().(PointerType).getBaseType()
else t = arg.getType().getUnderlyingType()
)
)
or
c = node.asIndirectExpr() and
(
// const char* means (const char)*, so the pointer is not const, the pointed to value is
// Grab the base type if a pointer, as this is the type we will check for const-ness
if c.getType().getUnderlyingType() instanceof PointerType
then t = c.getType().getUnderlyingType().(PointerType).getBaseType()
else t = c.getType().getUnderlyingType()
)
) and
node = callOutput(call, output) and
call.getStaticCallTarget() = func
) and
not exists(Call c |
c.getTarget().hasDefinition() and
if node instanceof DataFlow::DefinitionByReferenceNode
then c.getAnArgument() = node.asDefiningArgument()
else c = [node.asExpr(), node.asIndirectExpr()]
not exists(Function func, FunctionInput input, FunctionOutput output, CallInstruction call |
// NOTE: we must include dataflow and taintflow. e.g., including only dataflow we will find sprintf
// variant function's output are now possible non-const sources
(
func.(DataFlowFunction).hasDataFlow(input, output) or
func.(TaintFunction).hasTaintFlow(input, output)
) and
node = callOutput(call, output) and
call.getStaticCallTarget() = func
) and
not exists(Call c |
c.getTarget().hasDefinition() and
if node instanceof DataFlow::DefinitionByReferenceNode
then c.getAnArgument() = node.asDefiningArgument()
else c = [node.asExpr(), node.asIndirectExpr()]
)
)
}

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

@@ -1,15 +1,19 @@
| NonConstantFormat.c:30:10:30:16 | access to array | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| NonConstantFormat.c:41:9:41:27 | call to any_random_function | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| nested.cpp:21:23:21:26 | fmt0 | The format string argument to snprintf should be constant to prevent security issues and other potential errors. |
| nested.cpp:79:32:79:38 | call to get_fmt | The format string argument to diagnostic should be constant to prevent security issues and other potential errors. |
| NonConstantFormat.c:45:9:45:48 | call to gettext | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| nested.cpp:87:18:87:20 | fmt | The format string argument to diagnostic should be constant to prevent security issues and other potential errors. |
| test.cpp:51:10:51:21 | call to make_message | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:57:12:57:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:60:12:60:21 | call to const_wash | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:61:12:61:26 | ... + ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:62:12:62:17 | + ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:63:12:63:18 | * ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:64:12:64:18 | & ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:65:12:65:39 | ... + ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:130:20:130:26 | access to array | The format string argument to sprintf should be constant to prevent security issues and other potential errors. |
| test.cpp:157:12:157:15 | data | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:170:12:170:14 | res | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:195:31:195:33 | str | The format string argument to StringCchPrintfW should be constant to prevent security issues and other potential errors. |
| test.cpp:197:11:197:14 | wstr | The format string argument to wprintf should be constant to prevent security issues and other potential errors. |
| test.cpp:205:12:205:20 | ... + ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:206:12:206:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:211:12:211:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:217:12:217:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:223:12:223:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:228:12:228:18 | ++ ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:235:12:235:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:242:12:242:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:247:12:247:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |

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

@@ -18,7 +18,7 @@ extern "C" int snprintf ( char * s, int n, const char * format, ... );
struct A {
void do_print(const char *fmt0) {
char buf[32];
snprintf(buf, 32, fmt0); // BAD through call from c.do_some_printing(c.ext_fmt_str())
snprintf(buf, 32, fmt0); // GOOD, all paths to year use const char*
}
};
@@ -76,12 +76,12 @@ void diagnostic(const char *fmt, ...)
}
void bar(void) {
diagnostic (some_instance->get_fmt()); // BAD
diagnostic (some_instance->get_fmt()); // GOOD get_fmt is const char* assumed static
}
namespace ns {
class blab {
class blab {
void out1(void) {
char *fmt = (char *)__builtin_alloca(10);
diagnostic(fmt); // BAD

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

@@ -107,7 +107,7 @@ int main(int argc, char **argv) {
printf(hello); // GOOD
}
if (gettext_debug) {
printf(new char[100]); // BAD [FALSE NEGATIVE]
printf(new char[100]); // BAD [FALSE NEGATIVE: uninitialized value not considered a source]
}
{
const char *hello = "Hello, World\n";
@@ -197,7 +197,7 @@ void wchar_t_test_bad(wchar_t* str){
wprintf(wstr); // BAD
}
const char* get_string();
char* get_string();
void pointer_arithmetic_test_on_bad_string(){
{