From 173ade1336776a703da0806b95e141bc9406d179 Mon Sep 17 00:00:00 2001 From: Robert Marsh Date: Thu, 21 Feb 2019 15:49:53 -0800 Subject: [PATCH 1/4] C++: add arithmetic/bitwise instruction classes --- .../code/cpp/ir/implementation/Opcode.qll | 36 ++++++++++----- .../aliased_ssa/Instruction.qll | 44 ++++++++++++++----- .../cpp/ir/implementation/raw/Instruction.qll | 44 ++++++++++++++----- .../unaliased_ssa/Instruction.qll | 44 ++++++++++++++----- 4 files changed, 120 insertions(+), 48 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/Opcode.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/Opcode.qll index d2c258cf96e3..e6e9947c520c 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/Opcode.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/Opcode.qll @@ -85,6 +85,18 @@ abstract class PointerArithmeticOpcode extends BinaryOpcode {} abstract class PointerOffsetOpcode extends PointerArithmeticOpcode {} +abstract class ArithmeticOpcode extends Opcode {} + +abstract class BinaryArithmeticOpcode extends BinaryOpcode, ArithmeticOpcode {} + +abstract class UnaryArithmeticOpcode extends UnaryOpcode, ArithmeticOpcode {} + +abstract class BitwiseOpcode extends Opcode {} + +abstract class BinaryBitwiseOpcode extends BinaryOpcode, BitwiseOpcode {} + +abstract class UnaryBitwiseOpcode extends UnaryOpcode, BitwiseOpcode {} + abstract class CompareOpcode extends BinaryOpcode {} abstract class RelationalOpcode extends CompareOpcode {} @@ -143,18 +155,18 @@ module Opcode { class CopyValue extends UnaryOpcode, CopyOpcode, TCopyValue { override final string toString() { result = "CopyValue" } } class Load extends CopyOpcode, OpcodeWithLoad, TLoad { override final string toString() { result = "Load" } } class Store extends CopyOpcode, MemoryAccessOpcode, TStore { override final string toString() { result = "Store" } } - class Add extends BinaryOpcode, TAdd { override final string toString() { result = "Add" } } - class Sub extends BinaryOpcode, TSub { override final string toString() { result = "Sub" } } - class Mul extends BinaryOpcode, TMul { override final string toString() { result = "Mul" } } - class Div extends BinaryOpcode, TDiv { override final string toString() { result = "Div" } } - class Rem extends BinaryOpcode, TRem { override final string toString() { result = "Rem" } } - class Negate extends UnaryOpcode, TNegate { override final string toString() { result = "Negate" } } - class ShiftLeft extends BinaryOpcode, TShiftLeft { override final string toString() { result = "ShiftLeft" } } - class ShiftRight extends BinaryOpcode, TShiftRight { override final string toString() { result = "ShiftRight" } } - class BitAnd extends BinaryOpcode, TBitAnd { override final string toString() { result = "BitAnd" } } - class BitOr extends BinaryOpcode, TBitOr { override final string toString() { result = "BitOr" } } - class BitXor extends BinaryOpcode, TBitXor { override final string toString() { result = "BitXor" } } - class BitComplement extends UnaryOpcode, TBitComplement { override final string toString() { result = "BitComplement" } } + class Add extends BinaryArithmeticOpcode, TAdd { override final string toString() { result = "Add" } } + class Sub extends BinaryArithmeticOpcode, TSub { override final string toString() { result = "Sub" } } + class Mul extends BinaryArithmeticOpcode, TMul { override final string toString() { result = "Mul" } } + class Div extends BinaryArithmeticOpcode, TDiv { override final string toString() { result = "Div" } } + class Rem extends BinaryArithmeticOpcode, TRem { override final string toString() { result = "Rem" } } + class Negate extends UnaryArithmeticOpcode, TNegate { override final string toString() { result = "Negate" } } + class ShiftLeft extends BinaryBitwiseOpcode, TShiftLeft { override final string toString() { result = "ShiftLeft" } } + class ShiftRight extends BinaryBitwiseOpcode, TShiftRight { override final string toString() { result = "ShiftRight" } } + class BitAnd extends BinaryBitwiseOpcode, TBitAnd { override final string toString() { result = "BitAnd" } } + class BitOr extends BinaryBitwiseOpcode, TBitOr { override final string toString() { result = "BitOr" } } + class BitXor extends BinaryBitwiseOpcode, TBitXor { override final string toString() { result = "BitXor" } } + class BitComplement extends UnaryBitwiseOpcode, TBitComplement { override final string toString() { result = "BitComplement" } } class LogicalNot extends UnaryOpcode, TLogicalNot { override final string toString() { result = "LogicalNot" } } class CompareEQ extends CompareOpcode, TCompareEQ { override final string toString() { result = "CompareEQ" } } class CompareNE extends CompareOpcode, TCompareNE { override final string toString() { result = "CompareNE" } } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll index cb144033ab5b..fb25314f2300 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll @@ -898,67 +898,87 @@ class BinaryInstruction extends Instruction { } } -class AddInstruction extends BinaryInstruction { +class ArithmeticInstruction extends Instruction { + ArithmeticInstruction() { + getOpcode() instanceof ArithmeticOpcode + } +} + +class BinaryArithmeticInstruction extends ArithmeticInstruction, BinaryInstruction {} + +class UnaryArithmeticInstruction extends ArithmeticInstruction, UnaryInstruction {} + +class AddInstruction extends BinaryArithmeticInstruction { AddInstruction() { getOpcode() instanceof Opcode::Add } } -class SubInstruction extends BinaryInstruction { +class SubInstruction extends BinaryArithmeticInstruction { SubInstruction() { getOpcode() instanceof Opcode::Sub } } -class MulInstruction extends BinaryInstruction { +class MulInstruction extends BinaryArithmeticInstruction { MulInstruction() { getOpcode() instanceof Opcode::Mul } } -class DivInstruction extends BinaryInstruction { +class DivInstruction extends BinaryArithmeticInstruction { DivInstruction() { getOpcode() instanceof Opcode::Div } } -class RemInstruction extends BinaryInstruction { +class RemInstruction extends BinaryArithmeticInstruction { RemInstruction() { getOpcode() instanceof Opcode::Rem } } -class NegateInstruction extends UnaryInstruction { +class NegateInstruction extends UnaryArithmeticInstruction { NegateInstruction() { getOpcode() instanceof Opcode::Negate } } -class BitAndInstruction extends BinaryInstruction { +class BitwiseInstruction extends Instruction { + BitwiseInstruction() { + getOpcode() instanceof BitwiseOpcode + } +} + +class BinaryBitwiseInstruction extends BitwiseInstruction, BinaryInstruction {} + +class UnaryBitwiseInstruction extends BitwiseInstruction, UnaryInstruction {} + +class BitAndInstruction extends BinaryBitwiseInstruction { BitAndInstruction() { getOpcode() instanceof Opcode::BitAnd } } -class BitOrInstruction extends BinaryInstruction { +class BitOrInstruction extends BinaryBitwiseInstruction { BitOrInstruction() { getOpcode() instanceof Opcode::BitOr } } -class BitXorInstruction extends BinaryInstruction { +class BitXorInstruction extends BinaryBitwiseInstruction { BitXorInstruction() { getOpcode() instanceof Opcode::BitXor } } -class ShiftLeftInstruction extends BinaryInstruction { +class ShiftLeftInstruction extends BinaryBitwiseInstruction { ShiftLeftInstruction() { getOpcode() instanceof Opcode::ShiftLeft } } -class ShiftRightInstruction extends BinaryInstruction { +class ShiftRightInstruction extends BinaryBitwiseInstruction { ShiftRightInstruction() { getOpcode() instanceof Opcode::ShiftRight } @@ -1097,7 +1117,7 @@ class ConvertToDerivedInstruction extends InheritanceConversionInstruction { } } -class BitComplementInstruction extends UnaryInstruction { +class BitComplementInstruction extends UnaryBitwiseInstruction { BitComplementInstruction() { getOpcode() instanceof Opcode::BitComplement } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll index cb144033ab5b..fb25314f2300 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll @@ -898,67 +898,87 @@ class BinaryInstruction extends Instruction { } } -class AddInstruction extends BinaryInstruction { +class ArithmeticInstruction extends Instruction { + ArithmeticInstruction() { + getOpcode() instanceof ArithmeticOpcode + } +} + +class BinaryArithmeticInstruction extends ArithmeticInstruction, BinaryInstruction {} + +class UnaryArithmeticInstruction extends ArithmeticInstruction, UnaryInstruction {} + +class AddInstruction extends BinaryArithmeticInstruction { AddInstruction() { getOpcode() instanceof Opcode::Add } } -class SubInstruction extends BinaryInstruction { +class SubInstruction extends BinaryArithmeticInstruction { SubInstruction() { getOpcode() instanceof Opcode::Sub } } -class MulInstruction extends BinaryInstruction { +class MulInstruction extends BinaryArithmeticInstruction { MulInstruction() { getOpcode() instanceof Opcode::Mul } } -class DivInstruction extends BinaryInstruction { +class DivInstruction extends BinaryArithmeticInstruction { DivInstruction() { getOpcode() instanceof Opcode::Div } } -class RemInstruction extends BinaryInstruction { +class RemInstruction extends BinaryArithmeticInstruction { RemInstruction() { getOpcode() instanceof Opcode::Rem } } -class NegateInstruction extends UnaryInstruction { +class NegateInstruction extends UnaryArithmeticInstruction { NegateInstruction() { getOpcode() instanceof Opcode::Negate } } -class BitAndInstruction extends BinaryInstruction { +class BitwiseInstruction extends Instruction { + BitwiseInstruction() { + getOpcode() instanceof BitwiseOpcode + } +} + +class BinaryBitwiseInstruction extends BitwiseInstruction, BinaryInstruction {} + +class UnaryBitwiseInstruction extends BitwiseInstruction, UnaryInstruction {} + +class BitAndInstruction extends BinaryBitwiseInstruction { BitAndInstruction() { getOpcode() instanceof Opcode::BitAnd } } -class BitOrInstruction extends BinaryInstruction { +class BitOrInstruction extends BinaryBitwiseInstruction { BitOrInstruction() { getOpcode() instanceof Opcode::BitOr } } -class BitXorInstruction extends BinaryInstruction { +class BitXorInstruction extends BinaryBitwiseInstruction { BitXorInstruction() { getOpcode() instanceof Opcode::BitXor } } -class ShiftLeftInstruction extends BinaryInstruction { +class ShiftLeftInstruction extends BinaryBitwiseInstruction { ShiftLeftInstruction() { getOpcode() instanceof Opcode::ShiftLeft } } -class ShiftRightInstruction extends BinaryInstruction { +class ShiftRightInstruction extends BinaryBitwiseInstruction { ShiftRightInstruction() { getOpcode() instanceof Opcode::ShiftRight } @@ -1097,7 +1117,7 @@ class ConvertToDerivedInstruction extends InheritanceConversionInstruction { } } -class BitComplementInstruction extends UnaryInstruction { +class BitComplementInstruction extends UnaryBitwiseInstruction { BitComplementInstruction() { getOpcode() instanceof Opcode::BitComplement } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll index cb144033ab5b..fb25314f2300 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll @@ -898,67 +898,87 @@ class BinaryInstruction extends Instruction { } } -class AddInstruction extends BinaryInstruction { +class ArithmeticInstruction extends Instruction { + ArithmeticInstruction() { + getOpcode() instanceof ArithmeticOpcode + } +} + +class BinaryArithmeticInstruction extends ArithmeticInstruction, BinaryInstruction {} + +class UnaryArithmeticInstruction extends ArithmeticInstruction, UnaryInstruction {} + +class AddInstruction extends BinaryArithmeticInstruction { AddInstruction() { getOpcode() instanceof Opcode::Add } } -class SubInstruction extends BinaryInstruction { +class SubInstruction extends BinaryArithmeticInstruction { SubInstruction() { getOpcode() instanceof Opcode::Sub } } -class MulInstruction extends BinaryInstruction { +class MulInstruction extends BinaryArithmeticInstruction { MulInstruction() { getOpcode() instanceof Opcode::Mul } } -class DivInstruction extends BinaryInstruction { +class DivInstruction extends BinaryArithmeticInstruction { DivInstruction() { getOpcode() instanceof Opcode::Div } } -class RemInstruction extends BinaryInstruction { +class RemInstruction extends BinaryArithmeticInstruction { RemInstruction() { getOpcode() instanceof Opcode::Rem } } -class NegateInstruction extends UnaryInstruction { +class NegateInstruction extends UnaryArithmeticInstruction { NegateInstruction() { getOpcode() instanceof Opcode::Negate } } -class BitAndInstruction extends BinaryInstruction { +class BitwiseInstruction extends Instruction { + BitwiseInstruction() { + getOpcode() instanceof BitwiseOpcode + } +} + +class BinaryBitwiseInstruction extends BitwiseInstruction, BinaryInstruction {} + +class UnaryBitwiseInstruction extends BitwiseInstruction, UnaryInstruction {} + +class BitAndInstruction extends BinaryBitwiseInstruction { BitAndInstruction() { getOpcode() instanceof Opcode::BitAnd } } -class BitOrInstruction extends BinaryInstruction { +class BitOrInstruction extends BinaryBitwiseInstruction { BitOrInstruction() { getOpcode() instanceof Opcode::BitOr } } -class BitXorInstruction extends BinaryInstruction { +class BitXorInstruction extends BinaryBitwiseInstruction { BitXorInstruction() { getOpcode() instanceof Opcode::BitXor } } -class ShiftLeftInstruction extends BinaryInstruction { +class ShiftLeftInstruction extends BinaryBitwiseInstruction { ShiftLeftInstruction() { getOpcode() instanceof Opcode::ShiftLeft } } -class ShiftRightInstruction extends BinaryInstruction { +class ShiftRightInstruction extends BinaryBitwiseInstruction { ShiftRightInstruction() { getOpcode() instanceof Opcode::ShiftRight } @@ -1097,7 +1117,7 @@ class ConvertToDerivedInstruction extends InheritanceConversionInstruction { } } -class BitComplementInstruction extends UnaryInstruction { +class BitComplementInstruction extends UnaryBitwiseInstruction { BitComplementInstruction() { getOpcode() instanceof Opcode::BitComplement } From 9a9ec7bb17281366f4de504e73d7beae735a1473 Mon Sep 17 00:00:00 2001 From: Robert Marsh Date: Thu, 21 Feb 2019 15:51:10 -0800 Subject: [PATCH 2/4] C++: add IR-based taint tracking library --- .../code/cpp/ir/dataflow/TaintTracking.qll | 156 ++++++++++++++++++ .../taint-tests/IRTaintTestCommon.qll | 29 ++++ .../dataflow/taint-tests/TaintTestCommon.qll | 29 ++++ .../dataflow/taint-tests/taint.cpp | 2 +- .../dataflow/taint-tests/taint.ql | 30 +--- .../dataflow/taint-tests/test_diff.ql | 37 +++++ .../dataflow/taint-tests/test_ir.ql | 5 + 7 files changed, 258 insertions(+), 30 deletions(-) create mode 100644 cpp/ql/src/semmle/code/cpp/ir/dataflow/TaintTracking.qll create mode 100644 cpp/ql/test/library-tests/dataflow/taint-tests/IRTaintTestCommon.qll create mode 100644 cpp/ql/test/library-tests/dataflow/taint-tests/TaintTestCommon.qll create mode 100644 cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.ql create mode 100644 cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.ql diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/TaintTracking.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/TaintTracking.qll new file mode 100644 index 000000000000..26873ddd622b --- /dev/null +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/TaintTracking.qll @@ -0,0 +1,156 @@ +/** + * Provides classes for performing local (intra-procedural) and + * global (inter-procedural) taint-tracking analyses. + * + * We define _taint propagation_ informally to mean that a substantial part of + * the information from the source is preserved at the sink. For example, taint + * propagates from `x` to `x + 100`, but it does not propagate from `x` to `x > + * 100` since we consider a single bit of information to be too little. + */ + +import semmle.code.cpp.ir.dataflow.DataFlow +import semmle.code.cpp.ir.dataflow.DataFlow2 +private import semmle.code.cpp.ir.IR + +module TaintTracking { + /** + * A configuration of interprocedural taint tracking analysis. This defines + * sources, sinks, and any other configurable aspect of the analysis. Each + * use of the taint tracking library must define its own unique extension of + * this abstract class. + * + * A taint-tracking configuration is a special data flow configuration + * (`DataFlow::Configuration`) that allows for flow through nodes that do not + * necessarily preserve values but are still relevant from a taint-tracking + * perspective. (For example, string concatenation, where one of the operands + * is tainted.) + * + * To create a configuration, extend this class with a subclass whose + * characteristic predicate is a unique singleton string. For example, write + * + * ``` + * class MyAnalysisConfiguration extends TaintTracking::Configuration { + * MyAnalysisConfiguration() { this = "MyAnalysisConfiguration" } + * // Override `isSource` and `isSink`. + * // Optionally override `isSanitizer`. + * // Optionally override `isAdditionalTaintStep`. + * } + * ``` + * + * Then, to query whether there is flow between some `source` and `sink`, + * write + * + * ``` + * exists(MyAnalysisConfiguration cfg | cfg.hasFlow(source, sink)) + * ``` + * + * Multiple configurations can coexist, but it is unsupported to depend on a + * `TaintTracking::Configuration` or a `DataFlow::Configuration` in the + * overridden predicates that define sources, sinks, or additional steps. + * Instead, the dependency should go to a `TaintTracking::Configuration2` or + * a `DataFlow{2,3,4}::Configuration`. + */ + abstract class Configuration extends DataFlow::Configuration { + bindingset[this] + Configuration() { any() } + + /** Holds if `source` is a taint source. */ + // overridden to provide taint-tracking specific qldoc + abstract override predicate isSource(DataFlow::Node source); + + /** Holds if `sink` is a taint sink. */ + // overridden to provide taint-tracking specific qldoc + abstract override predicate isSink(DataFlow::Node sink); + + /** + * Holds if taint should not flow into `node`. + */ + predicate isSanitizer(DataFlow::Node node) { none() } + + /** + * Holds if the additional taint propagation step + * from `source` to `target` must be taken into account in the analysis. + * This step will only be followed if `target` is not in the `isSanitizer` + * predicate. + */ + predicate isAdditionalTaintStep(DataFlow::Node source, DataFlow::Node target) { none() } + + final override predicate isBarrier(DataFlow::Node node) { isSanitizer(node) } + + final override predicate isAdditionalFlowStep(DataFlow::Node source, DataFlow::Node target) { + this.isAdditionalTaintStep(source, target) + or + localTaintStep(source, target) + } + } + + /** + * A taint-tracking configuration that is backed by the `DataFlow2` library + * instead of `DataFlow`. Use this class when taint-tracking configurations + * or data-flow configurations must depend on each other. + * + * See `TaintTracking::Configuration` for the full documentation. + */ + abstract class Configuration2 extends DataFlow2::Configuration { + bindingset[this] + Configuration2() { any() } + + /** Holds if `source` is a taint source. */ + // overridden to provide taint-tracking specific qldoc + abstract override predicate isSource(DataFlow::Node source); + + /** Holds if `sink` is a taint sink. */ + // overridden to provide taint-tracking specific qldoc + abstract override predicate isSink(DataFlow::Node sink); + + /** + * Holds if taint should not flow into `node`. + */ + predicate isSanitizer(DataFlow::Node node) { none() } + + /** + * Holds if the additional taint propagation step + * from `source` to `target` must be taken into account in the analysis. + * This step will only be followed if `target` is not in the `isSanitizer` + * predicate. + */ + predicate isAdditionalTaintStep(DataFlow::Node source, DataFlow::Node target) { none() } + + final override predicate isBarrier(DataFlow::Node node) { isSanitizer(node) } + + final override predicate isAdditionalFlowStep(DataFlow::Node source, DataFlow::Node target) { + this.isAdditionalTaintStep(source, target) + or + localTaintStep(source, target) + } + } + + /** + * Holds if taint propagates from `nodeFrom` to `nodeTo` in exactly one local + * (intra-procedural) step. + */ + predicate localTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { + // Taint can flow into using ordinary data flow. + DataFlow::localFlowStep(nodeFrom, nodeTo) + or + // Taint can flow through expressions that alter the value but preserve + // more than one bit of it _or_ expressions that follow data through + // pointer indirections. + nodeTo.getAnOperand().getDefinitionInstruction() = nodeFrom and + ( + nodeTo instanceof ArithmeticInstruction + or + nodeTo instanceof BitwiseInstruction + or + nodeTo instanceof PointerArithmeticInstruction + or + nodeTo instanceof FieldAddressInstruction + ) + } + + /** + * Holds if taint may propagate from `source` to `sink` in zero or more local + * (intra-procedural) steps. + */ + predicate localTaint(DataFlow::Node source, DataFlow::Node sink) { localTaintStep*(source, sink) } +} diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/IRTaintTestCommon.qll b/cpp/ql/test/library-tests/dataflow/taint-tests/IRTaintTestCommon.qll new file mode 100644 index 000000000000..4cc98bf698f5 --- /dev/null +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/IRTaintTestCommon.qll @@ -0,0 +1,29 @@ +import cpp +import semmle.code.cpp.ir.dataflow.TaintTracking + +/** Common data flow configuration to be used by tests. */ +class TestAllocationConfig extends TaintTracking::Configuration { + TestAllocationConfig() { + this = "TestAllocationConfig" + } + + override predicate isSource(DataFlow::Node source) { + source.asExpr().(FunctionCall).getTarget().getName() = "source" + or + source.asParameter().getName().matches("source%") + or + // Track uninitialized variables + exists(source.asUninitialized()) + } + + override predicate isSink(DataFlow::Node sink) { + exists(FunctionCall call | + call.getTarget().getName() = "sink" and + sink.asExpr() = call.getAnArgument() + ) + } + + override predicate isSanitizer(DataFlow::Node barrier) { + barrier.asExpr().(VariableAccess).getTarget().hasName("sanitizer") + } +} diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/TaintTestCommon.qll b/cpp/ql/test/library-tests/dataflow/taint-tests/TaintTestCommon.qll new file mode 100644 index 000000000000..61130992e8be --- /dev/null +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/TaintTestCommon.qll @@ -0,0 +1,29 @@ +import cpp +import semmle.code.cpp.dataflow.TaintTracking + +/** Common data flow configuration to be used by tests. */ +class TestAllocationConfig extends TaintTracking::Configuration { + TestAllocationConfig() { + this = "TestAllocationConfig" + } + + override predicate isSource(DataFlow::Node source) { + source.asExpr().(FunctionCall).getTarget().getName() = "source" + or + source.asParameter().getName().matches("source%") + or + // Track uninitialized variables + exists(source.asUninitialized()) + } + + override predicate isSink(DataFlow::Node sink) { + exists(FunctionCall call | + call.getTarget().getName() = "sink" and + sink.asExpr() = call.getAnArgument() + ) + } + + override predicate isSanitizer(DataFlow::Node barrier) { + barrier.asExpr().(VariableAccess).getTarget().hasName("sanitizer") + } +} \ No newline at end of file diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp index d30514eedd4c..c9c028efe8a6 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp @@ -1,5 +1,5 @@ int source(); -void sink(...); +void sink(...) {}; void arithAssignments(int source1, int clean1) { sink(clean1); // clean diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.ql b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.ql index c35ddab6e1ec..455af5338cc5 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.ql +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.ql @@ -1,32 +1,4 @@ -import cpp -import semmle.code.cpp.dataflow.TaintTracking - -/** Common data flow configuration to be used by tests. */ -class TestAllocationConfig extends TaintTracking::Configuration { - TestAllocationConfig() { - this = "TestAllocationConfig" - } - - override predicate isSource(DataFlow::Node source) { - source.asExpr().(FunctionCall).getTarget().getName() = "source" - or - source.asParameter().getName().matches("source%") - or - // Track uninitialized variables - exists(source.asUninitialized()) - } - - override predicate isSink(DataFlow::Node sink) { - exists(FunctionCall call | - call.getTarget().getName() = "sink" and - sink.asExpr() = call.getAnArgument() - ) - } - - override predicate isSanitizer(DataFlow::Node barrier) { - barrier.asExpr().(VariableAccess).getTarget().hasName("sanitizer") - } -} +import TaintTestCommon from DataFlow::Node sink, DataFlow::Node source, TestAllocationConfig cfg where cfg.hasFlow(source, sink) diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.ql b/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.ql new file mode 100644 index 000000000000..39fddbac75dc --- /dev/null +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.ql @@ -0,0 +1,37 @@ +import cpp +import TaintTestCommon as ASTCommon +import IRTaintTestCommon as IRCommon +import semmle.code.cpp.dataflow.DataFlow as ASTDataFlow +import semmle.code.cpp.ir.dataflow.DataFlow as IRDataFlow + +predicate astFlow(Location sourceLocation, Location sinkLocation) { + exists(ASTDataFlow::DataFlow::Node source, ASTDataFlow::DataFlow::Node sink, + ASTCommon::TestAllocationConfig cfg | + cfg.hasFlow(source, sink) and + sourceLocation = source.getLocation() and + sinkLocation = sink.getLocation() + ) +} + +predicate irFlow(Location sourceLocation, Location sinkLocation) { + exists(IRDataFlow::DataFlow::Node source, IRDataFlow::DataFlow::Node sink, + IRCommon::TestAllocationConfig cfg | + cfg.hasFlow(source, sink) and + sourceLocation = source.getLocation() and + sinkLocation = sink.getLocation() + ) +} + +from Location sourceLocation, Location sinkLocation, string note +where + ( + astFlow(sourceLocation, sinkLocation) and + not irFlow(sourceLocation, sinkLocation) and + note = "AST only" + ) or + ( + irFlow(sourceLocation, sinkLocation) and + not astFlow(sourceLocation, sinkLocation) and + note = "IR only" + ) +select sourceLocation.toString(), sinkLocation.toString(), note diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.ql b/cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.ql new file mode 100644 index 000000000000..fa75216887de --- /dev/null +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.ql @@ -0,0 +1,5 @@ +import IRTaintTestCommon + +from DataFlow::Node sink, DataFlow::Node source, TestAllocationConfig cfg +where cfg.hasFlow(source, sink) +select sink, source From aa973026714ac5f7d96d86527826718e1d3c7661 Mon Sep 17 00:00:00 2001 From: Robert Marsh Date: Thu, 21 Feb 2019 17:17:49 -0800 Subject: [PATCH 3/4] make loads from tainted addresses tainted --- cpp/ql/src/semmle/code/cpp/ir/dataflow/TaintTracking.qll | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/TaintTracking.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/TaintTracking.qll index 26873ddd622b..d619e84e244d 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/TaintTracking.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/TaintTracking.qll @@ -146,6 +146,8 @@ module TaintTracking { or nodeTo instanceof FieldAddressInstruction ) + or + nodeTo.(LoadInstruction).getSourceAddress() = nodeFrom } /** From 07cbbdaf9adc52bda6840a1f8291eeac4c5898c2 Mon Sep 17 00:00:00 2001 From: Robert Marsh Date: Thu, 21 Feb 2019 17:18:06 -0800 Subject: [PATCH 4/4] C++: accept test output --- .../dataflow/taint-tests/test_diff.expected | 6 ++++++ .../library-tests/dataflow/taint-tests/test_ir.expected | 9 +++++++++ 2 files changed, 15 insertions(+) create mode 100644 cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected create mode 100644 cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.expected diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected new file mode 100644 index 000000000000..041c6f7920e0 --- /dev/null +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected @@ -0,0 +1,6 @@ +| taint.cpp:35:12:35:17 | taint.cpp:41:7:41:13 | AST only | +| taint.cpp:35:12:35:17 | taint.cpp:42:7:42:13 | AST only | +| taint.cpp:37:22:37:27 | taint.cpp:43:7:43:13 | AST only | +| taint.cpp:120:11:120:16 | taint.cpp:137:7:137:9 | AST only | +| taint.cpp:127:8:127:13 | taint.cpp:130:7:130:9 | IR only | +| taint.cpp:185:11:185:16 | taint.cpp:181:8:181:9 | AST only | diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.expected new file mode 100644 index 000000000000..aa3094506daf --- /dev/null +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.expected @@ -0,0 +1,9 @@ +| taint.cpp:8:8:8:13 | Load: clean1 | taint.cpp:4:27:4:33 | InitializeParameter: source1 | +| taint.cpp:16:8:16:14 | Load: source1 | taint.cpp:12:22:12:27 | Call: call to source | +| taint.cpp:17:8:17:16 | Add: ++ ... | taint.cpp:12:22:12:27 | Call: call to source | +| taint.cpp:129:7:129:9 | Load: * ... | taint.cpp:120:11:120:16 | Call: call to source | +| taint.cpp:130:7:130:9 | Load: * ... | taint.cpp:127:8:127:13 | Call: call to source | +| taint.cpp:134:7:134:9 | Load: * ... | taint.cpp:120:11:120:16 | Call: call to source | +| taint.cpp:151:7:151:12 | Call: call to select | taint.cpp:151:20:151:25 | Call: call to source | +| taint.cpp:167:8:167:13 | Call: call to source | taint.cpp:167:8:167:13 | Call: call to source | +| taint.cpp:168:8:168:14 | Load: tainted | taint.cpp:164:19:164:24 | Call: call to source |