diff --git a/change-notes/1.21/analysis-javascript.md b/change-notes/1.21/analysis-javascript.md index baa302f4d9bb..4515d1e67ab8 100644 --- a/change-notes/1.21/analysis-javascript.md +++ b/change-notes/1.21/analysis-javascript.md @@ -46,5 +46,6 @@ * `RegExpLiteral` is now a `DataFlow::SourceNode`. * `JSDocTypeExpr` now has source locations and is a subclass of `Locatable` and `TypeAnnotation`. +* The two-parameter versions of predicate `isBarrier` in `DataFlow::Configuration` and of predicate `isSanitizer` in `TaintTracking::Configuration` have been renamed to `isBarrierEdge` and `isSanitizerEdge`, respectively. The old names are maintained for backwards-compatibility in this version, but will be deprecated in the next version and subsequently removed. * Various predicates named `getTypeAnnotation()` now return `TypeAnnotation` instead of `TypeExpr`. In rare cases, this may cause compilation errors. Cast the result to `TypeExpr` if this happens. diff --git a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll index 8bd119b04189..2a16264463aa 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll @@ -154,15 +154,29 @@ abstract class Configuration extends string { } /** + * DEPRECATED: Use `isBarrierEdge` instead. + * * Holds if flow from `src` to `trg` is prohibited. */ predicate isBarrier(DataFlow::Node src, DataFlow::Node trg) { none() } /** + * DEPRECATED: Use `isBarrierEdge` instead. + * * Holds if flow with label `lbl` cannot flow from `src` to `trg`. */ predicate isBarrier(DataFlow::Node src, DataFlow::Node trg, FlowLabel lbl) { none() } + /** + * Holds if flow from `pred` to `succ` is prohibited. + */ + predicate isBarrierEdge(DataFlow::Node pred, DataFlow::Node succ) { none() } + + /** + * Holds if flow with label `lbl` cannot flow from `pred` to `succ`. + */ + predicate isBarrierEdge(DataFlow::Node pred, DataFlow::Node succ, FlowLabel lbl) { none() } + /** * Holds if flow with label `lbl` cannot flow into `node`. */ @@ -440,6 +454,7 @@ private predicate basicFlowStep( exists(FlowLabel predlbl, FlowLabel succlbl | localFlowStep(pred, succ, cfg, predlbl, succlbl) and not cfg.isBarrier(pred, succ, predlbl) and + not cfg.isBarrierEdge(pred, succ, predlbl) and summary = MkPathSummary(false, false, predlbl, succlbl) ) or @@ -553,7 +568,8 @@ private predicate callInputStep( ) ) and not cfg.isBarrier(succ) and - not cfg.isBarrier(pred, succ) + not cfg.isBarrier(pred, succ) and + not cfg.isBarrierEdge(pred, succ) } /** @@ -608,6 +624,7 @@ private predicate flowThroughCall( calls(output, f) and // Do not consider partial calls reachableFromInput(f, output, input, ret, cfg, summary) and not cfg.isBarrier(ret, output) and + not cfg.isBarrierEdge(ret, output) and not cfg.isLabeledBarrier(output, summary.getEndLabel()) ) or @@ -617,6 +634,7 @@ private predicate flowThroughCall( calls(invk, f) and reachableFromInput(f, invk, input, ret, cfg, summary) and not cfg.isBarrier(ret, output) and + not cfg.isBarrierEdge(ret, output) and not cfg.isLabeledBarrier(output, summary.getEndLabel()) ) } @@ -803,6 +821,7 @@ private predicate flowStep( ) and not cfg.isBarrier(succ) and not cfg.isBarrier(pred, succ) and + not cfg.isBarrierEdge(pred, succ) and not cfg.isLabeledBarrier(succ, summary.getEndLabel()) } diff --git a/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll b/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll index 1b80ada0ba31..72b03e97ff98 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll @@ -51,14 +51,30 @@ module TaintTracking { /** Holds if the intermediate node `node` is a taint sanitizer. */ predicate isSanitizer(DataFlow::Node node) { none() } - /** Holds if the edge from `source` to `sink` is a taint sanitizer. */ + /** + * DEPRECATED: Use `isSanitizerEdge` instead. + * + * Holds if the edge from `source` to `sink` is a taint sanitizer. + */ predicate isSanitizer(DataFlow::Node source, DataFlow::Node sink) { none() } - /** Holds if the edge from `source` to `sink` is a taint sanitizer for data labelled with `lbl`. */ + /** + * DEPRECATED: Use `isSanitizerEdge` instead. + * + * Holds if the edge from `source` to `sink` is a taint sanitizer for data labelled with `lbl`. + */ predicate isSanitizer(DataFlow::Node source, DataFlow::Node sink, DataFlow::FlowLabel lbl) { none() } + /** Holds if the edge from `pred` to `succ` is a taint sanitizer. */ + predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) { none() } + + /** Holds if the edge from `pred` to `succ` is a taint sanitizer for data labelled with `lbl`. */ + predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel lbl) { + none() + } + /** * Holds if data flow node `guard` can act as a sanitizer when appearing * in a condition. @@ -74,16 +90,18 @@ module TaintTracking { isSanitizer(node) } - final override predicate isBarrier(DataFlow::Node source, DataFlow::Node sink) { - super.isBarrier(source, sink) or - isSanitizer(source, sink) + final override predicate isBarrierEdge(DataFlow::Node source, DataFlow::Node sink) { + super.isBarrierEdge(source, sink) or + isSanitizer(source, sink) or + isSanitizerEdge(source, sink) } - final override predicate isBarrier( + final override predicate isBarrierEdge( DataFlow::Node source, DataFlow::Node sink, DataFlow::FlowLabel lbl ) { - super.isBarrier(source, sink, lbl) or - isSanitizer(source, sink, lbl) + super.isBarrierEdge(source, sink, lbl) or + isSanitizer(source, sink, lbl) or + isSanitizerEdge(source, sink, lbl) } final override predicate isBarrierGuard(DataFlow::BarrierGuardNode guard) { diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/ClientSideUrlRedirect.qll b/javascript/ql/src/semmle/javascript/security/dataflow/ClientSideUrlRedirect.qll index 177ab1cf4e5e..695ea05b4c8f 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/ClientSideUrlRedirect.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/ClientSideUrlRedirect.qll @@ -51,7 +51,7 @@ module ClientSideUrlRedirect { node instanceof Sanitizer } - override predicate isSanitizer(DataFlow::Node source, DataFlow::Node sink) { + override predicate isSanitizerEdge(DataFlow::Node source, DataFlow::Node sink) { hostnameSanitizingPrefixEdge(source, sink) } diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/InsecureRandomness.qll b/javascript/ql/src/semmle/javascript/security/dataflow/InsecureRandomness.qll index b66cabb786a4..307013bfb769 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/InsecureRandomness.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/InsecureRandomness.qll @@ -36,7 +36,7 @@ module InsecureRandomness { node instanceof Sanitizer } - override predicate isSanitizer(DataFlow::Node pred, DataFlow::Node succ) { + override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) { // stop propagation at the sinks to avoid double reporting pred instanceof Sink and // constrain succ diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/RequestForgery.qll b/javascript/ql/src/semmle/javascript/security/dataflow/RequestForgery.qll index 0e9da43f0fb7..101e819dbb1b 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/RequestForgery.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/RequestForgery.qll @@ -46,7 +46,7 @@ module RequestForgery { node instanceof Sanitizer } - override predicate isSanitizer(DataFlow::Node source, DataFlow::Node sink) { + override predicate isSanitizerEdge(DataFlow::Node source, DataFlow::Node sink) { sanitizingPrefixEdge(source, sink) } } diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/ServerSideUrlRedirect.qll b/javascript/ql/src/semmle/javascript/security/dataflow/ServerSideUrlRedirect.qll index e9941c15696e..da5be5e0ab27 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/ServerSideUrlRedirect.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/ServerSideUrlRedirect.qll @@ -38,7 +38,7 @@ module ServerSideUrlRedirect { node instanceof Sanitizer } - override predicate isSanitizer(DataFlow::Node source, DataFlow::Node sink) { + override predicate isSanitizerEdge(DataFlow::Node source, DataFlow::Node sink) { hostnameSanitizingPrefixEdge(source, sink) } diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/DataFlowConfig.qll b/javascript/ql/test/library-tests/InterProceduralFlow/DataFlowConfig.qll index dd66457474ff..7e7794961755 100644 --- a/javascript/ql/test/library-tests/InterProceduralFlow/DataFlowConfig.qll +++ b/javascript/ql/test/library-tests/InterProceduralFlow/DataFlowConfig.qll @@ -17,7 +17,7 @@ class TestDataFlowConfiguration extends DataFlow::Configuration { ) } - override predicate isBarrier(DataFlow::Node src, DataFlow::Node snk) { + override predicate isBarrierEdge(DataFlow::Node src, DataFlow::Node snk) { src = src and snk.asExpr().(PropAccess).getPropertyName() = "notTracked" or diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.ql b/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.ql index ead2b47825ea..f9b1a552086b 100644 --- a/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.ql +++ b/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.ql @@ -17,7 +17,7 @@ class TestTaintTrackingConfiguration extends TaintTracking::Configuration { ) } - override predicate isSanitizer(DataFlow::Node src, DataFlow::Node snk) { + override predicate isSanitizerEdge(DataFlow::Node src, DataFlow::Node snk) { src = src and snk.asExpr().(PropAccess).getPropertyName() = "notTracked" or