Commit 306440c
Lines changed: 32 additions & 67 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -12,79 +12,44 @@ | |||
| 12 | 12 | ||
| 13 | 13 | import cpp | |
| 14 | 14 | import semmle.code.cpp.commons.NullTermination | |
| 15 | - import semmle.code.cpp.ir.dataflow.internal.DefaultTaintTrackingImpl | ||
| 15 | + import semmle.code.cpp.security.FlowSources as FS | ||
| 16 | + import semmle.code.cpp.dataflow.new.TaintTracking | ||
| 17 | + import semmle.code.cpp.ir.IR | ||
| 16 | 18 | ||
| 17 | - /** A user-controlled expression that may not be null terminated. */ | ||
| 18 | - class TaintSource extends VariableAccess { | ||
| 19 | - TaintSource() { | ||
| 20 | - exists(SecurityOptions x, string cause | | ||
| 21 | - this.getTarget() instanceof SemanticStackVariable and | ||
| 22 | - x.isUserInput(this, cause) | ||
| 23 | - | | ||
| 24 | - cause = ["read", "fread", "recv", "recvfrom", "recvmsg"] | ||
| 25 | - ) | ||
| 26 | - } | ||
| 19 | + predicate isSource(FS::FlowSource source, string sourceType) { | ||
| 20 | + sourceType = source.getSourceType() and | ||
| 21 | + exists(VariableAccess va, Call call | | ||
| 22 | + va = source.asDefiningArgument() and | ||
| 23 | + call.getAnArgument() = va and | ||
| 24 | + va.getTarget() instanceof SemanticStackVariable and | ||
| 25 | + call.getTarget().hasGlobalName(["read", "fread", "recv", "recvfrom", "recvmsg"]) | ||
| 26 | + ) | ||
| 27 | + } | ||
| 27 | 28 | ||
| 28 | - /** | ||
| 29 | - * Holds if `sink` is a tainted variable access that must be null | ||
| 30 | - * terminated. | ||
| 31 | - */ | ||
| 32 | - private predicate isSink(VariableAccess sink) { | ||
| 33 | - tainted(this, sink) and | ||
| 34 | - variableMustBeNullTerminated(sink) | ||
| 35 | - } | ||
| 29 | + predicate isSink(DataFlow::Node sink, VariableAccess va) { | ||
| 30 | + va = [sink.asExpr(), sink.asIndirectExpr()] and | ||
| 31 | + variableMustBeNullTerminated(va) | ||
| 32 | + } | ||
| 36 | 33 | ||
| 37 | - /** | ||
| 38 | - * Holds if this source can reach `va`, possibly using intermediate | ||
| 39 | - * reassignments. | ||
| 40 | - */ | ||
| 41 | - private predicate sourceReaches(VariableAccess va) { | ||
| 42 | - definitionUsePair(_, this, va) | ||
| 43 | - or | ||
| 44 | - exists(VariableAccess mid, Expr def | | ||
| 45 | - this.sourceReaches(mid) and | ||
| 46 | - exprDefinition(_, def, mid) and | ||
| 47 | - definitionUsePair(_, def, va) | ||
| 48 | - ) | ||
| 49 | - } | ||
| 34 | + private module Config implements DataFlow::ConfigSig { | ||
| 35 | + predicate isSource(DataFlow::Node source) { isSource(source, _) } | ||
| 50 | 36 | ||
| 51 | - /** | ||
| 52 | - * Holds if the sink `sink` is reachable both from this source and | ||
| 53 | - * from `va`, possibly using intermediate reassignments. | ||
| 54 | - */ | ||
| 55 | - private predicate reachesSink(VariableAccess va, VariableAccess sink) { | ||
| 56 | - this.isSink(sink) and | ||
| 57 | - va = sink | ||
| 37 | + predicate isBarrier(DataFlow::Node node) { | ||
| 38 | + isSink(node) and node.asExpr().getUnspecifiedType() instanceof ArithmeticType | ||
| 39 | + or | ||
| 40 | + node.asInstruction().(StoreInstruction).getResultType() instanceof ArithmeticType | ||
| 58 | 41 | or | |
| 59 | - exists(VariableAccess mid, Expr def | | ||
| 60 | - this.reachesSink(mid, sink) and | ||
| 61 | - exprDefinition(_, def, va) and | ||
| 62 | - definitionUsePair(_, def, mid) | ||
| 63 | - ) | ||
| 42 | + mayAddNullTerminator(_, node.asIndirectExpr()) | ||
| 64 | 43 | } | |
| 65 | 44 | ||
| 66 | - /** | ||
| 67 | - * Holds if `sink` is a tainted variable access that must be null | ||
| 68 | - * terminated, and no access which null terminates its contents can | ||
| 69 | - * either reach the sink or be reached from the source. (Ideally, | ||
| 70 | - * we should instead look for such accesses only on the path from | ||
| 71 | - * this source to `sink` found via `tainted(source, sink)`.) | ||
| 72 | - */ | ||
| 73 | - predicate reaches(VariableAccess sink) { | ||
| 74 | - this.isSink(sink) and | ||
| 75 | - not exists(VariableAccess va | | ||
| 76 | - va != this and | ||
| 77 | - va != sink and | ||
| 78 | - mayAddNullTerminator(_, va) | ||
| 79 | - | | ||
| 80 | - this.sourceReaches(va) | ||
| 81 | - or | ||
| 82 | - this.reachesSink(va, sink) | ||
| 83 | - ) | ||
| 84 | - } | ||
| 45 | + predicate isSink(DataFlow::Node sink) { isSink(sink, _) } | ||
| 85 | 46 | } | |
| 86 | 47 | ||
| 87 | - from TaintSource source, VariableAccess sink | ||
| 88 | - where source.reaches(sink) | ||
| 89 | - select sink, "String operation depends on a $@ that may not be null terminated.", source, | ||
| 90 | - "user-provided value" | ||
| 48 | + module Flow = TaintTracking::Global<Config>; | ||
| 49 | + | ||
| 50 | + from DataFlow::Node source, DataFlow::Node sink, VariableAccess va, string sourceType | ||
| 51 | + where | ||
| 52 | + Flow::flow(source, sink) and | ||
| 53 | + isSource(source, sourceType) and | ||
| 54 | + isSink(sink, va) | ||
| 55 | + select va, "String operation depends on a $@ that may not be null terminated.", source, sourceType | ||
0 commit comments