Commit 078f223
Lines changed: 77 additions & 10 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -15,7 +15,11 @@ | |||
| 15 | 15 | ||
| 16 | 16 | import cpp | |
| 17 | 17 | import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis | |
| 18 | - import semmle.code.cpp.ir.dataflow.internal.DefaultTaintTrackingImpl | ||
| 18 | + import semmle.code.cpp.dataflow.new.DataFlow | ||
| 19 | + import semmle.code.cpp.security.FlowSources as FS | ||
| 20 | + import semmle.code.cpp.dataflow.new.TaintTracking | ||
| 21 | + import semmle.code.cpp.ir.IR | ||
| 22 | + import semmle.code.cpp.controlflow.IRGuards as IRGuards | ||
| 19 | 23 | ||
| 20 | 24 | /** Holds if `expr` might overflow. */ | |
| 21 | 25 | predicate outOfBoundsExpr(Expr expr, string kind) { | |
@@ -27,13 +31,76 @@ predicate outOfBoundsExpr(Expr expr, string kind) { | |||
| 27 | 31 | else none() | |
| 28 | 32 | } | |
| 29 | 33 | ||
| 30 | - from Expr use, Expr origin, string kind | ||
| 34 | + predicate isSource(FS::FlowSource source, string sourceType) { sourceType = source.getSourceType() } | ||
| 35 | + | ||
| 36 | + predicate isSink(DataFlow::Node sink, string kind) { | ||
| 37 | + exists(Expr use | | ||
| 38 | + use = sink.asExpr() and | ||
| 39 | + not use.getUnspecifiedType() instanceof PointerType and | ||
| 40 | + outOfBoundsExpr(use, kind) and | ||
| 41 | + not inSystemMacroExpansion(use) | ||
| 42 | + ) | ||
| 43 | + } | ||
| 44 | + | ||
| 45 | + predicate hasUpperBoundsCheck(Variable var) { | ||
| 46 | + exists(RelationalOperation oper, VariableAccess access | | ||
| 47 | + oper.getAnOperand() = access and | ||
| 48 | + access.getTarget() = var and | ||
| 49 | + // Comparing to 0 is not an upper bound check | ||
| 50 | + not oper.getAnOperand().getValue() = "0" | ||
| 51 | + ) | ||
| 52 | + } | ||
| 53 | + | ||
| 54 | + predicate constantInstruction(Instruction instr) { | ||
| 55 | + instr instanceof ConstantInstruction or | ||
| 56 | + constantInstruction(instr.(UnaryInstruction).getUnary()) | ||
| 57 | + } | ||
| 58 | + | ||
| 59 | + predicate readsVariable(LoadInstruction load, Variable var) { | ||
| 60 | + load.getSourceAddress().(VariableAddressInstruction).getAstVariable() = var | ||
| 61 | + } | ||
| 62 | + | ||
| 63 | + predicate nodeIsBarrierEqualityCandidate(DataFlow::Node node, Operand access, Variable checkedVar) { | ||
| 64 | + exists(Instruction instr | instr = node.asInstruction() | | ||
| 65 | + readsVariable(instr, checkedVar) and | ||
| 66 | + any(IRGuards::IRGuardCondition guard).ensuresEq(access, _, _, instr.getBlock(), true) | ||
| 67 | + ) | ||
| 68 | + } | ||
| 69 | + | ||
| 70 | + module Config implements DataFlow::ConfigSig { | ||
| 71 | + predicate isSource(DataFlow::Node source) { isSource(source, _) } | ||
| 72 | + | ||
| 73 | + predicate isSink(DataFlow::Node sink) { isSink(sink, _) } | ||
| 74 | + | ||
| 75 | + predicate isBarrier(DataFlow::Node node) { | ||
| 76 | + // Block flow if there's an upper bound check of the variable anywhere in the program | ||
| 77 | + exists(Variable checkedVar, Instruction instr | instr = node.asInstruction() | | ||
| 78 | + readsVariable(instr, checkedVar) and | ||
| 79 | + hasUpperBoundsCheck(checkedVar) | ||
| 80 | + ) | ||
| 81 | + or | ||
| 82 | + // Block flow if the node is guarded by an equality check | ||
| 83 | + exists(Variable checkedVar, Operand access | | ||
| 84 | + nodeIsBarrierEqualityCandidate(node, access, checkedVar) and | ||
| 85 | + readsVariable(access.getDef(), checkedVar) | ||
| 86 | + ) | ||
| 87 | + or | ||
| 88 | + // Block flow to any binary instruction whose operands are both non-constants. | ||
| 89 | + exists(BinaryInstruction iTo | | ||
| 90 | + iTo = node.asInstruction() and | ||
| 91 | + not constantInstruction(iTo.getLeft()) and | ||
| 92 | + not constantInstruction(iTo.getRight()) and | ||
| 93 | + // propagate taint from either the pointer or the offset, regardless of constantness | ||
| 94 | + not iTo instanceof PointerArithmeticInstruction | ||
| 95 | + ) | ||
| 96 | + } | ||
| 97 | + } | ||
| 98 | + | ||
| 99 | + module Flow = TaintTracking::Global<Config>; | ||
| 100 | + | ||
| 101 | + from DataFlow::Node source, DataFlow::Node sink, string kind, string sourceType | ||
| 31 | 102 | where | |
| 32 | - not use.getUnspecifiedType() instanceof PointerType and | ||
| 33 | - outOfBoundsExpr(use, kind) and | ||
| 34 | - tainted(origin, use) and | ||
| 35 | - origin != use and | ||
| 36 | - not inSystemMacroExpansion(use) and | ||
| 37 | - // Avoid double-counting: don't include all the conversions of `use`. | ||
| 38 | - not use instanceof Conversion | ||
| 39 | - select use, "$@ flows an expression which might " + kind + ".", origin, "User-provided value" | ||
| 103 | + Flow::flow(source, sink) and | ||
| 104 | + isSource(source, sourceType) and | ||
| 105 | + isSink(sink, kind) | ||
| 106 | + select sink, "$@ flows an expression which might " + kind + ".", source, sourceType | ||
0 commit comments