hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-16 08:43:11 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3788 from geoffw0/callderef

Browse Source 
C++: Add bcopy to models and use it.
This commit is contained in:
Mathias Vorreiter Pedersen
2020-10-20 12:15:23 +02:00
committed by GitHub
parent 4cc7138784 f9987cff64
commit 528afc55ab
7 changed files with 130 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
change-notes/1.26/analysis-cpp.md
View File

@@ -26,5 +26,6 @@ The following changes in version 1.26 affect C/C++ analysis in all applications.
* The models library now models many taint flows through `std::istream` and `std::ostream`.
* The models library now models some taint flows through `std::shared_ptr`, `std::unique_ptr`, `std::make_shared` and `std::make_unique`.
* The models library now models many taint flows through `std::pair`, `std::map`, `std::unordered_map`, `std::set` and `std::unordered_set`.
* The models library now models `bcopy`.
* The `SimpleRangeAnalysis` library now supports multiplications of the form
`e1 * e2` and `x *= e2` when `e1` and `e2` are unsigned or constant.

44
cpp/ql/src/semmle/code/cpp/controlflow/Dereferenced.qll
View File

@@ -4,47 +4,25 @@
import cpp
import Nullness
import semmle.code.cpp.models.interfaces.ArrayFunction
/**
* Holds if the call `fc` will dereference argument `i`.
*/
predicate callDereferences(FunctionCall fc, int i) {
exists(string name |
fc.getTarget().hasGlobalOrStdName(name) and
exists(ArrayFunction af |
fc.getTarget() = af and
(
name = "bcopy" and i in [0 .. 1]
or
name = "memcpy" and i in [0 .. 1]
or
name = "memmove" and i in [0 .. 1]
or
name = "strcpy" and i in [0 .. 1]
or
name = "strncpy" and i in [0 .. 1]
or
name = "strdup" and i = 0
or
name = "strndup" and i = 0
or
name = "strlen" and i = 0
or
name = "printf" and fc.getArgument(i).getType() instanceof PointerType
or
name = "fprintf" and fc.getArgument(i).getType() instanceof PointerType
or
name = "sprintf" and fc.getArgument(i).getType() instanceof PointerType
or
name = "snprintf" and fc.getArgument(i).getType() instanceof PointerType
or
name = "vprintf" and fc.getArgument(i).getType() instanceof PointerType
or
name = "vfprintf" and fc.getArgument(i).getType() instanceof PointerType
or
name = "vsprintf" and fc.getArgument(i).getType() instanceof PointerType
or
name = "vsnprintf" and fc.getArgument(i).getType() instanceof PointerType
af.hasArrayInput(i) or
af.hasArrayOutput(i)
)
)
or
exists(FormattingFunction ff |
fc.getTarget() = ff and
i >= ff.getFirstFormatArgumentIndex() and
fc.getArgument(i).getType() instanceof PointerType
)
}
/**

68
cpp/ql/src/semmle/code/cpp/models/implementations/Memcpy.qll
View File

@@ -10,45 +10,57 @@ import semmle.code.cpp.models.interfaces.SideEffect
import semmle.code.cpp.models.interfaces.Taint
/**
* The standard functions `memcpy` and `memmove`, and the gcc variant
* `__builtin___memcpy_chk`
* The standard functions `memcpy`, `memmove` and `bcopy`; and the gcc variant
* `__builtin___memcpy_chk`.
*/
class MemcpyFunction extends ArrayFunction, DataFlowFunction, SideEffectFunction, TaintFunction {
class MemcpyFunction extends ArrayFunction, DataFlowFunction, SideEffectFunction {
MemcpyFunction() {
this.hasName("memcpy") or
this.hasName("memmove") or
this.hasName("__builtin___memcpy_chk")
// memcpy(dest, src, num)
// memmove(dest, src, num)
// memmove(dest, src, num, remaining)
this.hasName(["memcpy", "memmove", "__builtin___memcpy_chk"])
or
// bcopy(src, dest, num)
this.hasGlobalOrStdName("bcopy")
}
override predicate hasArrayInput(int bufParam) { bufParam = 1 }
/**
* Gets the index of the parameter that is the source buffer for the copy.
*/
int getParamSrc() { if this.hasGlobalOrStdName("bcopy") then result = 0 else result = 1 }
override predicate hasArrayOutput(int bufParam) { bufParam = 0 }
/**
* Gets the index of the parameter that is the destination buffer for the
* copy.
*/
int getParamDest() { if this.hasGlobalOrStdName("bcopy") then result = 1 else result = 0 }
/**
* Gets the index of the parameter that is the size of the copy (in bytes).
*/
int getParamSize() { result = 2 }
override predicate hasArrayInput(int bufParam) { bufParam = getParamSrc() }
override predicate hasArrayOutput(int bufParam) { bufParam = getParamDest() }
override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
input.isParameterDeref(1) and
output.isParameterDeref(0)
input.isParameterDeref(getParamSrc()) and
output.isParameterDeref(getParamDest())
or
input.isParameterDeref(1) and
input.isParameterDeref(getParamSrc()) and
output.isReturnValueDeref()
or
input.isParameter(0) and
input.isParameter(getParamDest()) and
output.isReturnValue()
}
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
input.isParameter(2) and
output.isParameterDeref(0)
or
input.isParameter(2) and
output.isReturnValueDeref()
}
override predicate hasArrayWithVariableSize(int bufParam, int countParam) {
(
bufParam = 0 or
bufParam = 1
bufParam = getParamDest() or
bufParam = getParamSrc()
) and
countParam = 2
countParam = getParamSize()
}
override predicate hasOnlySpecificReadSideEffects() { any() }
@@ -56,18 +68,18 @@ class MemcpyFunction extends ArrayFunction, DataFlowFunction, SideEffectFunction
override predicate hasOnlySpecificWriteSideEffects() { any() }
override predicate hasSpecificWriteSideEffect(ParameterIndex i, boolean buffer, boolean mustWrite) {
i = 0 and buffer = true and mustWrite = true
i = getParamDest() and buffer = true and mustWrite = true
}
override predicate hasSpecificReadSideEffect(ParameterIndex i, boolean buffer) {
i = 1 and buffer = true
i = getParamSrc() and buffer = true
}
override ParameterIndex getParameterSizeIndex(ParameterIndex i) {
result = 2 and
result = getParamSize() and
(
i = 0 or
i = 1
i = getParamDest() or
i = getParamSrc()
)
}
}

2
cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
View File

@@ -5507,8 +5507,6 @@
| taint.cpp:194:10:194:10 | x | taint.cpp:194:9:194:10 | & ... | |
| taint.cpp:194:13:194:18 | source | taint.cpp:194:2:194:7 | call to memcpy | TAINT |
| taint.cpp:194:13:194:18 | source | taint.cpp:194:9:194:10 | ref arg & ... | TAINT |
| taint.cpp:194:21:194:31 | sizeof(int) | taint.cpp:194:2:194:7 | call to memcpy | TAINT |
| taint.cpp:194:21:194:31 | sizeof(int) | taint.cpp:194:9:194:10 | ref arg & ... | TAINT |
| taint.cpp:207:6:207:11 | call to source | taint.cpp:207:2:207:13 | ... = ... | |
| taint.cpp:207:6:207:11 | call to source | taint.cpp:210:7:210:7 | x | |
| taint.cpp:207:6:207:11 | call to source | taint.cpp:213:12:213:12 | x | |

6
cpp/ql/test/query-tests/Critical/MissingNullTest/MissingNullTest.expected Normal file
View File

@@ -0,0 +1,6 @@
| test.cpp:23:8:23:8 | p | Value may be null; it should be checked before dereferencing. |
| test.cpp:35:10:35:10 | q | Value may be null; it should be checked before dereferencing. |
| test.cpp:43:13:43:13 | q | Value may be null; it should be checked before dereferencing. |
| test.cpp:51:17:51:17 | q | Value may be null; it should be checked before dereferencing. |
| test.cpp:58:8:58:8 | p | Value may be null; it should be checked before dereferencing. |
| test.cpp:67:8:67:8 | p | Value may be null; it should be checked before dereferencing. |

1
cpp/ql/test/query-tests/Critical/MissingNullTest/MissingNullTest.qlref Normal file
View File

@@ -0,0 +1 @@
Critical/MissingNullTest.ql

71
cpp/ql/test/query-tests/Critical/MissingNullTest/test.cpp Normal file
View File

@@ -0,0 +1,71 @@
#define NULL (0)
typedef unsigned long size_t;
void *memcpy(void *s1, const void *s2, size_t n);
void bcopy(const void *source, void *dest, size_t amount);
void mycopyint(const int *source, int *dest)
{
*dest = *source;
}
void test1(bool cond)
{
int x, y;
{
int *p, *q;
y = *p; // BAD (p is uninitialized and could be 0) [NOT DETECTED]
p = NULL;
y = *p; // BAD (p is 0)
p = &x;
y = *p; // GOOD (p points to x)
p = q;
y = *p; // BAD (p is uninitialized and could be 0) [NOT DETECTED]
}
{
int *p = &x;
int *q = 0;
memcpy(p, &y, sizeof(int)); // GOOD (p points to x)
memcpy(q, &y, sizeof(int)); // BAD (p is 0)
}
{
int *p = &x;
int *q = 0;
bcopy(&y, p, sizeof(int)); // GOOD (p points to x)
bcopy(&y, q, sizeof(int)); // BAD (p is 0)
}
{
int *p = &x;
int *q = 0;
mycopyint(&y, p); // GOOD (p points to x)
mycopyint(&y, q); // BAD (p is 0)
}
{
int *p = 0;
int *q = &x;
y = *p; // BAD (p is 0)
memcpy(&p, &q, sizeof(p));
y = *p; // GOOD (p points to x)
}
{
int *p = 0;
int *q = &x;
y = *p; // BAD (p is 0)
bcopy(&q, &p, sizeof(p));
y = *p; // GOOD (p points to x)
}
}