CSG @ GovTech
Published in

CSG @ GovTech

Removing False Positives in CodeQL

Introduction

Memcpy Overflows

void * memcpy ( void * destination, const void * source, size_t num );
import cpp
import semmle.code.cpp.dataflow.TaintTracking
import DataFlow::PathGraph

class Config extends TaintTracking::Configuration {
Config() { this = “MemcpySink” }

override predicate isSource(DataFlow::Node source) {
exists(Expr E |
source.asExpr() = E
)
}

override predicate isSink(DataFlow::Node sink) {
exists(FunctionCall fc | fc.getTarget().hasName(“memcpy”) and sink.asExpr() = fc.getArgument(2))
and not sink.asExpr().isConstant()
}
}

from Config cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where cfg.hasFlowPath(source, sink)
select sink, source, sink, “All source expressions to memcpy “ + source

An Attempt to Rule out False Positives

void* malloc (size_t size);

class MallocSize extends Expr {
MallocSize () {
exists(
FunctionCall fc |
fc.getTarget().hasName(“malloc”) and
this = fc.getArgument(0)
)
}
}

override predicate isSink(DataFlow::Node sink) {
exists(FunctionCall fc, MallocSize mc |
fc.getTarget().hasName(“memcpy”) and
fc.getArgument(2).(VariableAccess).getTarget() = mc.(VariableAccess).getTarget() and
sink.asExpr() = fc.getArgument(2))
and not sink.asExpr().isConstant()
}
import cpp
import semmle.code.cpp.dataflow.TaintTracking
import DataFlow::PathGraph


// Finds all non-constant operands of arithmetic operations
class MallocSize extends Expr {
MallocSize () {
exists(
FunctionCall fc |
fc.getTarget().hasName(“malloc”) and
this = fc.getArgument(0)
)
}
}

class Config extends TaintTracking::Configuration {
Config() { this = “MallocToMemcpySink” }

override predicate isSource(DataFlow::Node source) {
exists(Expr E |
source.asExpr() = E
and not E.isConstant()
)
}

override predicate isSink(DataFlow::Node sink) {
exists(FunctionCall fc, MallocSize mc |
fc.getTarget().hasName(“memcpy”) and
fc.getArgument(2).(VariableAccess).getTarget() = mc.(VariableAccess).getTarget() and
sink.asExpr() = fc.getArgument(2))
and not sink.asExpr().isConstant()
}
}


from Config cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where cfg.hasFlowPath(source, sink)
select sink, source, sink, “any source flows to memcpy where size is operand in malloc”

Expanding the Filter’s Scope

class RecursiveBinaryArithmeticOperation extends BinaryArithmeticOperation {
Expr recursiveGetAnOperand() {
(result = this.getAnOperand() or
(
this.getAnOperand() instanceof BinaryArithmeticOperation and
result = this.getAnOperand().(RecursiveBinaryArithmeticOperation).recursiveGetAnOperand()
)) and not
result instanceof BinaryArithmeticOperation and not
result.isConstant()
}
}
class MallocSize extends Expr {
MallocSize () {
exists(
FunctionCall fc |
fc.getTarget().hasName(“malloc”) and
fc.getArgument(0) instanceof RecursiveBinaryArithmeticOperation and
this = fc.getArgument(0).(RecursiveBinaryArithmeticOperation).recursiveGetAnOperand()
)
}
}
import cpp
import semmle.code.cpp.dataflow.TaintTracking
import DataFlow::PathGraph


// Adds recursive getAnOperand
class RecursiveBinaryArithmeticOperation extends BinaryArithmeticOperation {
Expr recursiveGetAnOperand() {
(result = this.getAnOperand() or
(
this.getAnOperand() instanceof BinaryArithmeticOperation and
result = this.getAnOperand().(RecursiveBinaryArithmeticOperation).recursiveGetAnOperand()
)) and not
result instanceof BinaryArithmeticOperation and not
result.isConstant()
}
}

// Finds all non-constant operands of arithmetic operations
class MallocSize extends Expr {
MallocSize () {
exists(
FunctionCall fc |
fc.getTarget().hasName(“malloc”) and
fc.getArgument(0) instanceof RecursiveBinaryArithmeticOperation and
this = fc.getArgument(0).(RecursiveBinaryArithmeticOperation).recursiveGetAnOperand()
)
}
}

class Config extends TaintTracking::Configuration {
Config() { this = “RecvUserInputToSink” }

override predicate isSource(DataFlow::Node source) {
exists(Expr E |
source.asExpr() = E
and not E.isConstant()
)
}

override predicate isSink(DataFlow::Node sink) {
exists(FunctionCall fc, MallocSize mc |
fc.getTarget().hasName(“memcpy”) and not
fc.getArgument(2).(VariableAccess).getTarget() = mc.(VariableAccess).getTarget() and
sink.asExpr() = fc.getArgument(2))
and not sink.asExpr().isConstant()
}
}


from Config cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where cfg.hasFlowPath(source, sink)
select sink, source, sink, “any source flows to memcpy where size is operand in malloc”

Improvements

Closing Off

--

--

Get the Medium app

A button that says 'Download on the App Store', and if clicked it will lead you to the iOS App store
A button that says 'Get it on, Google Play', and if clicked it will lead you to the Google Play store