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

C++: Port the 'predictable' barrier from 'DefaultTaintTracking' to 'cpp/unclear-array-index-validation' to prevent an explosion of new results.

Browse Source
This commit is contained in:
Mathias Vorreiter Pedersen
2022-03-14 13:14:07 +00:00
parent 0da5d91955
commit 50b77761f1
1 changed files with 33 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
cpp/ql/src/Security/CWE/CWE-129/ImproperArrayIndexValidation.ql
View File

@@ -18,6 +18,7 @@ import semmle.code.cpp.security.FlowSources
import semmle.code.cpp.ir.dataflow.TaintTracking
import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
import DataFlow::PathGraph
import semmle.code.cpp.security.Security
predicate hasUpperBound(VariableAccess offsetExpr) {
exists(BasicBlock controlled, StackVariable offsetVar, SsaDefinition def |
@@ -55,6 +56,15 @@ predicate nodeIsBarrierEqualityCandidate(DataFlow::Node node, Operand access, Va
predicate isFlowSource(FlowSource source, string sourceType) { sourceType = source.getSourceType() }
predicate predictableInstruction(Instruction instr) {
instr instanceof ConstantInstruction
or
instr instanceof StringConstantInstruction
or
// This could be a conversion on a string literal
predictableInstruction(instr.(UnaryInstruction).getUnary())
}
class ImproperArrayIndexValidationConfig extends TaintTracking::Configuration {
ImproperArrayIndexValidationConfig() { this = "ImproperArrayIndexValidationConfig" }
@@ -63,6 +73,8 @@ class ImproperArrayIndexValidationConfig extends TaintTracking::Configuration {
override predicate isSanitizer(DataFlow::Node node) {
hasUpperBound(node.asExpr())
or
// These barriers are ported from `DefaultTaintTracking` because this query is quite noisy
// otherwise.
exists(Variable checkedVar |
readsVariable(node.asInstruction(), checkedVar) and
hasUpperBoundsCheck(checkedVar)
@@ -72,6 +84,27 @@ class ImproperArrayIndexValidationConfig extends TaintTracking::Configuration {
readsVariable(access.getDef(), checkedVar) and
nodeIsBarrierEqualityCandidate(node, access, checkedVar)
)
or
// Don't use dataflow into binary instructions if both operands are unpredictable
exists(BinaryInstruction iTo |
iTo = node.asInstruction() and
not predictableInstruction(iTo.getLeft()) and
not predictableInstruction(iTo.getRight()) and
// propagate taint from either the pointer or the offset, regardless of predictability
not iTo instanceof PointerArithmeticInstruction
)
or
// don't use dataflow through calls to pure functions if two or more operands
// are unpredictable
exists(Instruction iFrom1, Instruction iFrom2, CallInstruction iTo |
iTo = node.asInstruction() and
isPureFunction(iTo.getStaticCallTarget().getName()) and
iFrom1 = iTo.getAnArgument() and
iFrom2 = iTo.getAnArgument() and
not predictableInstruction(iFrom1) and
not predictableInstruction(iFrom2) and
iFrom1 != iFrom2
)
}
override predicate isSink(DataFlow::Node sink) {