From f8804943ee78286c72fbc169cb77e2d49039ae4f Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Mon, 5 Aug 2019 12:05:12 +0200 Subject: [PATCH] Java: Change in/out barriers to be explicit in the configuration. --- .../CWE-190/ArithmeticWithExtremeValues.ql | 6 ++--- .../code/java/dataflow/TaintTracking.qll | 24 +++++++++++++++++-- .../java/dataflow/internal/DataFlowImpl.qll | 20 ++++++++++++---- .../dataflow/inoutbarriers/test.ql | 8 ++++--- 4 files changed, 45 insertions(+), 13 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-190/ArithmeticWithExtremeValues.ql b/java/ql/src/Security/CWE/CWE-190/ArithmeticWithExtremeValues.ql index b7a956ec9f1..46818b3efff 100644 --- a/java/ql/src/Security/CWE/CWE-190/ArithmeticWithExtremeValues.ql +++ b/java/ql/src/Security/CWE/CWE-190/ArithmeticWithExtremeValues.ql @@ -40,9 +40,9 @@ class ExtremeSourceFlowConfig extends DataFlow::Configuration { override predicate isSink(DataFlow::Node sink) { sink(_, sink.asExpr()) } - override predicate isBarrier(DataFlow::Node n) { - n.getType() instanceof BooleanType or isSource(n) - } + override predicate isBarrierIn(DataFlow::Node n) { isSource(n) } + + override predicate isBarrier(DataFlow::Node n) { n.getType() instanceof BooleanType } } predicate sink(ArithExpr exp, VarAccess use) { diff --git a/java/ql/src/semmle/code/java/dataflow/TaintTracking.qll b/java/ql/src/semmle/code/java/dataflow/TaintTracking.qll index b2ec74e5efc..d1829c43815 100644 --- a/java/ql/src/semmle/code/java/dataflow/TaintTracking.qll +++ b/java/ql/src/semmle/code/java/dataflow/TaintTracking.qll @@ -63,13 +63,23 @@ module TaintTracking { node.asExpr() instanceof ValidatedVariableAccess } - /** DEPRECATED: override `isSanitizer` instead. */ + /** DEPRECATED: override `isSanitizerIn` and `isSanitizerOut` instead. */ deprecated predicate isSanitizerEdge(DataFlow::Node node1, DataFlow::Node node2) { none() } deprecated final override predicate isBarrierEdge(DataFlow::Node node1, DataFlow::Node node2) { isSanitizerEdge(node1, node2) } + /** Holds if data flow into `node` is prohibited. */ + predicate isSanitizerIn(DataFlow::Node node) { none() } + + final override predicate isBarrierIn(DataFlow::Node node) { isSanitizerIn(node) } + + /** Holds if data flow out of `node` is prohibited. */ + predicate isSanitizerOut(DataFlow::Node node) { none() } + + final override predicate isBarrierOut(DataFlow::Node node) { isSanitizerOut(node) } + /** * Holds if the additional taint propagation step from `node1` to `node2` * must be taken into account in the analysis. @@ -135,13 +145,23 @@ module TaintTracking { node.asExpr() instanceof ValidatedVariableAccess } - /** DEPRECATED: override `isSanitizer` instead. */ + /** DEPRECATED: override `isSanitizerIn` and `isSanitizerOut` instead. */ deprecated predicate isSanitizerEdge(DataFlow::Node node1, DataFlow::Node node2) { none() } deprecated final override predicate isBarrierEdge(DataFlow::Node node1, DataFlow::Node node2) { isSanitizerEdge(node1, node2) } + /** Holds if data flow into `node` is prohibited. */ + predicate isSanitizerIn(DataFlow::Node node) { none() } + + final override predicate isBarrierIn(DataFlow::Node node) { isSanitizerIn(node) } + + /** Holds if data flow out of `node` is prohibited. */ + predicate isSanitizerOut(DataFlow::Node node) { none() } + + final override predicate isBarrierOut(DataFlow::Node node) { isSanitizerOut(node) } + /** * Holds if the additional taint propagation step from `node1` to `node2` * must be taken into account in the analysis. diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll index cfe85663344..d6dccd45656 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll @@ -57,9 +57,15 @@ abstract class Configuration extends string { /** Holds if data flow through `node` is prohibited. */ predicate isBarrier(Node node) { none() } - /** DEPRECATED: override `isBarrier` instead. */ + /** DEPRECATED: override `isBarrierIn` and `isBarrierOut` instead. */ deprecated predicate isBarrierEdge(Node node1, Node node2) { none() } + /** Holds if data flow into `node` is prohibited. */ + predicate isBarrierIn(Node node) { none() } + + /** Holds if data flow out of `node` is prohibited. */ + predicate isBarrierOut(Node node) { none() } + /** * Holds if the additional flow step from `node1` to `node2` must be taken * into account in the analysis. @@ -104,18 +110,22 @@ abstract class Configuration extends string { } private predicate inBarrier(Node node, Configuration config) { - config.isBarrier(node) and + config.isBarrierIn(node) and config.isSource(node) } private predicate outBarrier(Node node, Configuration config) { - config.isBarrier(node) and + config.isBarrierOut(node) and config.isSink(node) } private predicate fullBarrier(Node node, Configuration config) { - config.isBarrier(node) and - not config.isSource(node) and + config.isBarrier(node) + or + config.isBarrierIn(node) and + not config.isSource(node) + or + config.isBarrierOut(node) and not config.isSink(node) } diff --git a/java/ql/test/library-tests/dataflow/inoutbarriers/test.ql b/java/ql/test/library-tests/dataflow/inoutbarriers/test.ql index 1b21c9efb25..03c1290b172 100644 --- a/java/ql/test/library-tests/dataflow/inoutbarriers/test.ql +++ b/java/ql/test/library-tests/dataflow/inoutbarriers/test.ql @@ -29,7 +29,7 @@ class Conf2 extends Configuration { override predicate isSink(Node n) { sink0(n) } - override predicate isBarrier(Node n) { src0(n) } + override predicate isBarrierIn(Node n) { src0(n) } } class Conf3 extends Configuration { @@ -39,7 +39,7 @@ class Conf3 extends Configuration { override predicate isSink(Node n) { sink0(n) } - override predicate isBarrier(Node n) { sink0(n) } + override predicate isBarrierOut(Node n) { sink0(n) } } class Conf4 extends Configuration { @@ -49,7 +49,9 @@ class Conf4 extends Configuration { override predicate isSink(Node n) { sink0(n) } - override predicate isBarrier(Node n) { src0(n) or sink0(n) } + override predicate isBarrierIn(Node n) { src0(n) } + + override predicate isBarrierOut(Node n) { sink0(n) } } predicate flow(Node src, Node sink, string s) {