CPP: Make some repairs manually.

This commit is contained in:
Geoffrey White
2019-04-17 14:31:26 +01:00
parent e395f5215f
commit 6234b26496
6 changed files with 33 additions and 11 deletions

View File

@@ -26,18 +26,20 @@ class MinusOne extends NullValue {
*/
predicate mayCallFunction(Expr call, Function f) {
call.(FunctionCall).getTarget() = f or
call.(VariableCall).getVariable().getAnAssignedValue().getAChild*().(FunctionAccess).getTarget() =
f
call.(VariableCall).getVariable().getAnAssignedValue().
getAChild*().(FunctionAccess).getTarget() = f
}
predicate fopenCallOrIndirect(Expr e) {
// direct fopen call
fopenCall(e) and
// We are only interested in fopen calls that are
// actually closed somehow, as FileNeverClosed
// will catch those that aren't.
fopenCallMayBeClosed(e)
or
exists(ReturnStmt rtn |
// indirect fopen call
mayCallFunction(e, rtn.getEnclosingFunction()) and
@@ -84,6 +86,7 @@ class FOpenVariableReachability extends LocalScopeVariableReachabilityWithReassi
exists(node.(AnalysedExpr).getNullSuccessor(v)) or
fcloseCallOrIndirect(node, v) or
assignedToFieldOrGlobal(v, node) or
// node may be used directly in query
v.getFunction() = node.(ReturnStmt).getEnclosingFunction()
}
@@ -119,10 +122,12 @@ class FOpenReachability extends LocalScopeVariableReachabilityExt {
}
override predicate isBarrier(
ControlFlowNode source, ControlFlowNode node, ControlFlowNode next, LocalScopeVariable v
) {
ControlFlowNode source, ControlFlowNode node, ControlFlowNode next,
LocalScopeVariable v)
{
isSource(source, v) and
next = node.getASuccessor() and
// the file (stored in any variable `v0`) opened at `source` is closed or
// assigned to a global at node, or NULL checked on the edge node -> next.
exists(LocalScopeVariable v0 | fopenVariableReaches(v0, source, node) |
@@ -167,4 +172,6 @@ where
fopenVariableReaches(v, def, ret) and
ret.getAChild*() = v.getAnAccess()
)
select def, "The file opened here may not be closed at $@.", ret, "this exit point"
select
def, "The file opened here may not be closed at $@.",
ret, "this exit point"

View File

@@ -22,4 +22,7 @@ class FreedExpr extends PointsToExpr {
override predicate interesting() { freed(this) }
}
predicate allocMayBeFreed(Expr alloc) { isAllocationExpr(alloc) and anythingPointsTo(alloc) }
predicate allocMayBeFreed(Expr alloc) {
isAllocationExpr(alloc) and
anythingPointsTo(alloc)
}

View File

@@ -18,18 +18,20 @@ import semmle.code.cpp.controlflow.LocalScopeVariableReachability
*/
predicate mayCallFunction(Expr call, Function f) {
call.(FunctionCall).getTarget() = f or
call.(VariableCall).getVariable().getAnAssignedValue().getAChild*().(FunctionAccess).getTarget() =
f
call.(VariableCall).getVariable().getAnAssignedValue().
getAChild*().(FunctionAccess).getTarget() = f
}
predicate allocCallOrIndirect(Expr e) {
// direct alloc call
isAllocationExpr(e) and
// We are only interested in alloc calls that are
// actually freed somehow, as MemoryNeverFreed
// will catch those that aren't.
allocMayBeFreed(e)
or
exists(ReturnStmt rtn |
// indirect alloc call
mayCallFunction(e, rtn.getEnclosingFunction()) and
@@ -62,6 +64,7 @@ predicate verifiedRealloc(FunctionCall reallocCall, Variable v, ControlFlowNode
newV.getAnAssignedValue() = reallocCall and
node.(AnalysedExpr).getNonNullSuccessor(newV) = verified and
// note: this case uses naive flow logic (getAnAssignedValue).
// special case: if the result of the 'realloc' is assigned to the
// same variable, we don't descriminate properly between the old
// and the new allocation; better to not consider this a free at
@@ -113,6 +116,7 @@ class AllocVariableReachability extends LocalScopeVariableReachabilityWithReassi
exists(node.(AnalysedExpr).getNullSuccessor(v)) or
freeCallOrIndirect(node, v) or
assignedToFieldOrGlobal(v, node) or
// node may be used directly in query
v.getFunction() = node.(ReturnStmt).getEnclosingFunction()
}
@@ -148,10 +152,12 @@ class AllocReachability extends LocalScopeVariableReachabilityExt {
}
override predicate isBarrier(
ControlFlowNode source, ControlFlowNode node, ControlFlowNode next, LocalScopeVariable v
) {
ControlFlowNode source, ControlFlowNode node, ControlFlowNode next,
LocalScopeVariable v)
{
isSource(source, v) and
next = node.getASuccessor() and
// the memory (stored in any variable `v0`) allocated at `source` is freed or
// assigned to a global at node, or NULL checked on the edge node -> next.
exists(LocalScopeVariable v0 | allocatedVariableReaches(v0, source, node) |
@@ -196,4 +202,6 @@ where
allocatedVariableReaches(v, def, ret) and
ret.getAChild*() = v.getAnAccess()
)
select def, "The memory allocated here may not be released at $@.", ret, "this exit point"
select
def, "The memory allocated here may not be released at $@.",
ret, "this exit point"

View File

@@ -11,6 +11,7 @@
import cpp
// See also InitialisationNotRun.ql and GlobalUseBeforeInit.ql
/**
* Holds if `s` defines variable `v` (conservative).
*/

View File

@@ -33,10 +33,12 @@ predicate sourceSized(FunctionCall fc, Expr src) {
fc.getArgument(2) = size and
src = v.getAnAccess() and
size.getAChild+() = v.getAnAccess() and
// exception: `dest` is also referenced in the size argument
not exists(Variable other |
dest = other.getAnAccess() and size.getAChild+() = other.getAnAccess()
) and
// exception: `src` and `dest` are both arrays of the same type and size
not exists(ArrayType srctype, ArrayType desttype |
dest.getType().getUnderlyingType() = desttype and

View File

@@ -33,6 +33,7 @@ class BufferAccess extends ArrayExpr {
staticBuffer(this.getArrayBase(), _, size) and
size != 0
) and
// exclude accesses in macro implementation of `strcmp`,
// which are carefully controlled but can look dangerous.
not exists(Macro m |