mirror of
https://github.com/github/codeql.git
synced 2025-12-27 06:06:32 +01:00
97 lines
3.4 KiB
Plaintext
97 lines
3.4 KiB
Plaintext
/**
|
|
* @name Copy function using source size
|
|
* @description Calling a copy operation with a size derived from the source
|
|
* buffer instead of the destination buffer may result in a buffer overflow.
|
|
* @kind path-problem
|
|
* @id cpp/overflow-destination
|
|
* @problem.severity warning
|
|
* @security-severity 9.3
|
|
* @precision low
|
|
* @tags reliability
|
|
* security
|
|
* external/cwe/cwe-119
|
|
* external/cwe/cwe-131
|
|
*/
|
|
|
|
import cpp
|
|
import semmle.code.cpp.ir.dataflow.TaintTracking
|
|
import semmle.code.cpp.controlflow.IRGuards
|
|
import semmle.code.cpp.security.FlowSources
|
|
import DataFlow::PathGraph
|
|
|
|
/**
|
|
* Holds if `fc` is a call to a copy operation where the size argument contains
|
|
* a reference to the source argument. For example:
|
|
* ```
|
|
* memcpy(dest, src, sizeof(src));
|
|
* ```
|
|
*/
|
|
predicate sourceSized(FunctionCall fc, Expr src) {
|
|
fc.getTarget().hasGlobalOrStdName(["strncpy", "strncat", "memcpy", "memmove"]) and
|
|
exists(Expr dest, Expr size, Variable v |
|
|
fc.getArgument(0) = dest and
|
|
fc.getArgument(1).getFullyConverted() = src and
|
|
fc.getArgument(2) = size and
|
|
src = v.getAnAccess().getFullyConverted() 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
|
|
src.getType().getUnderlyingType() = srctype and
|
|
desttype.getBaseType().getUnderlyingType() = srctype.getBaseType().getUnderlyingType() and
|
|
desttype.getArraySize() = srctype.getArraySize()
|
|
)
|
|
)
|
|
}
|
|
|
|
predicate readsVariable(LoadInstruction load, Variable var) {
|
|
load.getSourceAddress().(VariableAddressInstruction).getAstVariable() = var
|
|
}
|
|
|
|
predicate hasUpperBoundsCheck(Variable var) {
|
|
exists(RelationalOperation oper, VariableAccess access |
|
|
oper.getAnOperand() = access and
|
|
access.getTarget() = var and
|
|
// Comparing to 0 is not an upper bound check
|
|
not oper.getAnOperand().getValue() = "0"
|
|
)
|
|
}
|
|
|
|
predicate nodeIsBarrierEqualityCandidate(DataFlow::Node node, Operand access, Variable checkedVar) {
|
|
readsVariable(node.asInstruction(), checkedVar) and
|
|
any(IRGuardCondition guard).ensuresEq(access, _, _, node.asInstruction().getBlock(), true)
|
|
}
|
|
|
|
class OverflowDestinationConfig extends TaintTracking::Configuration {
|
|
OverflowDestinationConfig() { this = "OverflowDestinationConfig" }
|
|
|
|
override predicate isSource(DataFlow::Node source) { source instanceof FlowSource }
|
|
|
|
override predicate isSink(DataFlow::Node sink) { sourceSized(_, sink.asConvertedExpr()) }
|
|
|
|
override predicate isSanitizer(DataFlow::Node node) {
|
|
exists(Variable checkedVar |
|
|
readsVariable(node.asInstruction(), checkedVar) and
|
|
hasUpperBoundsCheck(checkedVar)
|
|
)
|
|
or
|
|
exists(Variable checkedVar, Operand access |
|
|
readsVariable(access.getDef(), checkedVar) and
|
|
nodeIsBarrierEqualityCandidate(node, access, checkedVar)
|
|
)
|
|
}
|
|
}
|
|
|
|
from
|
|
FunctionCall fc, OverflowDestinationConfig conf, DataFlow::PathNode source,
|
|
DataFlow::PathNode sink
|
|
where
|
|
conf.hasFlowPath(source, sink) and
|
|
sourceSized(fc, sink.getNode().asConvertedExpr())
|
|
select fc, source, sink,
|
|
"To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size."
|