From a8fdd759f9c0193106e1f0f95e6aeab2bd7c9041 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 11 Dec 2024 09:47:22 +0100 Subject: [PATCH 01/37] JS: Add FlowState class to TaintedUrlSuffix --- .../TaintedUrlSuffixCustomizations.qll | 51 ++++++++++++++++++- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll index af69ede9197b..6708f9b3c8ec 100644 --- a/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll @@ -1,5 +1,5 @@ /** - * Provides a flow label for reasoning about URLs with a tainted query and fragment part, + * Provides a flow state for reasoning about URLs with a tainted query and fragment part, * which we collectively refer to as the "suffix" of the URL. */ @@ -7,12 +7,59 @@ import javascript private import semmle.javascript.dataflow.internal.DataFlowPrivate as DataFlowPrivate /** - * Provides a flow label for reasoning about URLs with a tainted query and fragment part, + * Provides a flow state for reasoning about URLs with a tainted query and fragment part, * which we collectively refer to as the "suffix" of the URL. */ module TaintedUrlSuffix { private import DataFlow + private newtype TFlowState = + TTaint() or + TTaintedUrlSuffix() + + /** + * A flow state with two values, `taint` and `tainted-url-suffix`. + * + * The `tainted-url-suffix` state represents a URL with a tainted query and fragment part, + * which we collectively refer to as the "suffix" of the URL. + * + * The `taint` state corresponds to ordinary taint. + */ + class FlowState extends TFlowState { + /** + * Holds if this represents a value that is considered entirely tainted. + */ + predicate isTaint() { this = TTaint() } + + /** + * Holds if this represents a URL whose fragment and/or query parts are considered tainted. + */ + predicate isTaintedUrlSuffix() { this = TTaintedUrlSuffix() } + + /** Gets a string representation of this flow state. */ + string toString() { + this.isTaint() and result = "taint" + or + this.isTaintedUrlSuffix() and result = "tainted-url-suffix" + } + + /** DEPRECATED. Gets the corresponding flow label. */ + deprecated DataFlow::FlowLabel toFlowLabel() { + this.isTaint() and result.isTaint() + or + this.isTaintedUrlSuffix() and result instanceof TaintedUrlSuffixLabel + } + } + + /** Convenience predicates for working with flow states. */ + module FlowState { + /** Gets the `taint` flow state. */ + FlowState taint() { result.isTaint() } + + /** Gets the `tainted-url-suffix` flow state. */ + FlowState taintedUrlSuffix() { result.isTaintedUrlSuffix() } + } + /** * The flow label representing a URL with a tainted query and fragment part. * From cca980298f1a21a9b8d13c9d75df137c68372fe7 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 11 Dec 2024 09:58:37 +0100 Subject: [PATCH 02/37] JS: Use flow state in barrier and step relations --- .../javascript/security/TaintedUrlSuffix.qll | 2 +- .../TaintedUrlSuffixCustomizations.qll | 38 ++++++++++++++----- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffix.qll b/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffix.qll index c55e2f5004ab..1d4ff0c4b7fe 100644 --- a/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffix.qll +++ b/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffix.qll @@ -12,7 +12,7 @@ import javascript module TaintedUrlSuffix { import TaintedUrlSuffixCustomizations::TaintedUrlSuffix - private class ConcreteTaintedUrlSuffixLabel extends TaintedUrlSuffixLabel { + deprecated private class ConcreteTaintedUrlSuffixLabel extends TaintedUrlSuffixLabel { ConcreteTaintedUrlSuffixLabel() { this = this } } } diff --git a/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll index 6708f9b3c8ec..7da1503a830e 100644 --- a/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll @@ -58,6 +58,9 @@ module TaintedUrlSuffix { /** Gets the `tainted-url-suffix` flow state. */ FlowState taintedUrlSuffix() { result.isTaintedUrlSuffix() } + + /** DEPRECATED. Gets the flow state correpsonding to `label`. */ + deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } } /** @@ -65,14 +68,14 @@ module TaintedUrlSuffix { * * Can also be accessed using `TaintedUrlSuffix::label()`. */ - abstract class TaintedUrlSuffixLabel extends FlowLabel { + abstract deprecated class TaintedUrlSuffixLabel extends FlowLabel { TaintedUrlSuffixLabel() { this = "tainted-url-suffix" } } /** * Gets the flow label representing a URL with a tainted query and fragment part. */ - FlowLabel label() { result instanceof TaintedUrlSuffixLabel } + deprecated FlowLabel label() { result instanceof TaintedUrlSuffixLabel } /** Gets a remote flow source that is a tainted URL query or fragment part from `window.location`. */ ClientSideRemoteFlowSource source() { @@ -84,22 +87,39 @@ module TaintedUrlSuffix { } /** + * DEPRECATED. Use `isStateBarrier(node, state)` instead. + * * Holds if `node` should be a barrier for the given `label`. * * This should be used in the `isBarrier` predicate of a configuration that uses the tainted-url-suffix * label. */ - predicate isBarrier(Node node, FlowLabel label) { - label = label() and - DataFlowPrivate::optionalBarrier(node, "split-url-suffix") + deprecated predicate isBarrier(Node node, FlowLabel label) { + isStateBarrier(node, FlowState::fromFlowLabel(label)) + } + + /** + * Holds if `node` should be blocked in `state`. + */ + predicate isStateBarrier(Node node, FlowState state) { + DataFlowPrivate::optionalBarrier(node, "split-url-suffix") and + state.isTaintedUrlSuffix() + } + + /** + * DEPRECATED. Use `isAdditionalFlowStep` instead. + */ + deprecated predicate step(Node src, Node dst, FlowLabel srclbl, FlowLabel dstlbl) { + isAdditionalFlowStep(src, FlowState::fromFlowLabel(srclbl), dst, + FlowState::fromFlowLabel(dstlbl)) } /** - * Holds if there is a flow step `src -> dst` involving the URL suffix taint label. + * Holds if there is a flow step `src -> dst` involving the URL suffix flow state. * * This handles steps through string operations, promises, URL parsers, and URL accessors. */ - predicate step(Node src, Node dst, FlowLabel srclbl, FlowLabel dstlbl) { + predicate isAdditionalFlowStep(Node src, FlowState srclbl, Node dst, FlowState dstlbl) { // Transition from tainted-url-suffix to general taint when entering the second array element // of a split('#') or split('?') array. // @@ -108,12 +128,12 @@ module TaintedUrlSuffix { // Technically we should also preverse tainted-url-suffix when entering the first array element of such // a split, but this mostly leads to FPs since we currently don't track if the taint has been through URI-decoding. // (The query/fragment parts are often URI-decoded in practice, but not the other URL parts are not) - srclbl = label() and + srclbl.isTaintedUrlSuffix() and dstlbl.isTaint() and DataFlowPrivate::optionalStep(src, "split-url-suffix-post", dst) or // Transition from URL suffix to full taint when extracting the query/fragment part. - srclbl = label() and + srclbl.isTaintedUrlSuffix() and dstlbl.isTaint() and ( exists(MethodCallNode call, string name | From 3cf14d8506f2af24455fd0b12fb573e6c12efe64 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 11 Dec 2024 10:04:44 +0100 Subject: [PATCH 03/37] JS: Migrate ClientSideUrlRedirect to flow state --- .../ClientSideUrlRedirectCustomizations.qll | 17 +++++++++----- .../dataflow/ClientSideUrlRedirectQuery.qll | 22 +++++++++---------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll index 3601a8114545..836a501f418d 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll @@ -8,12 +8,19 @@ import javascript private import semmle.javascript.security.TaintedUrlSuffixCustomizations module ClientSideUrlRedirect { + class FlowState = TaintedUrlSuffix::FlowState; + + module FlowState = TaintedUrlSuffix::FlowState; + /** * A data flow source for unvalidated URL redirect vulnerabilities. */ abstract class Source extends DataFlow::Node { - /** Gets a flow label to associate with this source. */ - DataFlow::FlowLabel getAFlowLabel() { result.isTaint() } + /** Gets a flow state to associate with this source. */ + FlowState getAFlowState() { result.isTaint() } + + /** DEPRECATED. Use `getAFlowState()` instead. */ + deprecated DataFlow::FlowLabel getAFlowLabel() { result = this.getAFlowState().toFlowLabel() } } /** @@ -50,10 +57,8 @@ module ClientSideUrlRedirect { private class ActiveThreatModelSourceAsSource extends Source instanceof ActiveThreatModelSource { ActiveThreatModelSourceAsSource() { not this.(ClientSideRemoteFlowSource).getKind().isPath() } - override DataFlow::FlowLabel getAFlowLabel() { - if this = TaintedUrlSuffix::source() - then result = TaintedUrlSuffix::label() - else result.isTaint() + override FlowState getAFlowState() { + if this = TaintedUrlSuffix::source() then result.isTaintedUrlSuffix() else result.isTaint() } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectQuery.qll index f703fd6a81d2..9a6cff968fb5 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectQuery.qll @@ -21,13 +21,13 @@ deprecated private class ConcreteDocumentUrl extends DocumentUrl { * A taint-tracking configuration for reasoning about unvalidated URL redirections. */ module ClientSideUrlRedirectConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + class FlowState = TaintedUrlSuffix::FlowState; - predicate isSource(DataFlow::Node source, DataFlow::FlowLabel state) { - source.(Source).getAFlowLabel() = state + predicate isSource(DataFlow::Node source, FlowState state) { + source.(Source).getAFlowState() = state } - predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel state) { + predicate isSink(DataFlow::Node sink, FlowState state) { sink instanceof Sink and state.isTaint() } @@ -35,19 +35,18 @@ module ClientSideUrlRedirectConfig implements DataFlow::StateConfigSig { node instanceof Sanitizer or node = HostnameSanitizerGuard::getABarrierNode() } - predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel state) { - TaintedUrlSuffix::isBarrier(node, state) + predicate isBarrier(DataFlow::Node node, FlowState state) { + TaintedUrlSuffix::isStateBarrier(node, state) } predicate isBarrierOut(DataFlow::Node node) { hostnameSanitizingPrefixEdge(node, _) } - predicate isBarrierOut(DataFlow::Node node, DataFlow::FlowLabel label) { isSink(node, label) } + predicate isBarrierOut(DataFlow::Node node, FlowState label) { isSink(node, label) } predicate isAdditionalFlowStep( - DataFlow::Node node1, DataFlow::FlowLabel state1, DataFlow::Node node2, - DataFlow::FlowLabel state2 + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - TaintedUrlSuffix::step(node1, node2, state1, state2) + TaintedUrlSuffix::isAdditionalFlowStep(node1, state1, node2, state2) or exists(HtmlSanitizerCall call | node1 = call.getInput() and @@ -85,7 +84,8 @@ deprecated class Configuration extends TaintTracking::Configuration { DataFlow::Node node1, DataFlow::Node node2, DataFlow::FlowLabel state1, DataFlow::FlowLabel state2 ) { - ClientSideUrlRedirectConfig::isAdditionalFlowStep(node1, state1, node2, state2) + ClientSideUrlRedirectConfig::isAdditionalFlowStep(node1, FlowState::fromFlowLabel(state1), + node2, FlowState::fromFlowLabel(state2)) or // Preserve document.url label in step from `location` to `location.href` or `location.toString()` state1 instanceof DocumentUrl and From 114d4a141a41770d33bbaacedd4106d099027135 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 11 Dec 2024 11:03:59 +0100 Subject: [PATCH 04/37] JS: Move FlowState definition into CommonFlowState Needed for migrating the XSS query --- .../javascript/security/CommonFlowState.qll | 61 +++++++++++++++++++ .../TaintedUrlSuffixCustomizations.qll | 51 +--------------- .../ClientSideUrlRedirectCustomizations.qll | 4 +- .../dataflow/ClientSideUrlRedirectQuery.qll | 2 +- 4 files changed, 64 insertions(+), 54 deletions(-) create mode 100644 javascript/ql/lib/semmle/javascript/security/CommonFlowState.qll diff --git a/javascript/ql/lib/semmle/javascript/security/CommonFlowState.qll b/javascript/ql/lib/semmle/javascript/security/CommonFlowState.qll new file mode 100644 index 000000000000..1d4e8746312e --- /dev/null +++ b/javascript/ql/lib/semmle/javascript/security/CommonFlowState.qll @@ -0,0 +1,61 @@ +/** + * Contains a class with flow states that are used by multiple queries. + */ + +private import javascript +private import TaintedUrlSuffixCustomizations + +private newtype TFlowState = + TTaint() or + TTaintedUrlSuffix() or + +/** + * A flow state indicating which part of a value is tainted. + */ +class FlowState extends TFlowState { + /** + * Holds if this represents a value that is considered entirely tainted, except the first character + * might not be user-controlled. + */ + predicate isTaint() { this = TTaint() } + + /** + * Holds if this represents a URL whose fragment and/or query parts are considered tainted. + */ + predicate isTaintedUrlSuffix() { this = TTaintedUrlSuffix() } + + /** Gets a string representation of this flow state. */ + string toString() { + this.isTaint() and result = "taint" + or + this.isTaintedUrlSuffix() and result = "tainted-url-suffix" + or + this.isTaintedPrefix() and result = "tainted-prefix" + } + + /** DEPRECATED. Gets the corresponding flow label. */ + deprecated DataFlow::FlowLabel toFlowLabel() { + this.isTaint() and result.isTaint() + or + this.isTaintedUrlSuffix() and result = TaintedUrlSuffix::label() + or + this.isTaintedPrefix() and result = "PrefixString" + } +} + +/** Convenience predicates for working with common flow states. */ +module FlowState { + /** + * Gets the flow state representing a value that is considered entirely tainted, except the first character + * might not be user-controlled. + */ + FlowState taint() { result.isTaint() } + + /** + * Gets the flow state representing a URL whose fragment and/or query parts are considered tainted. + */ + FlowState taintedUrlSuffix() { result.isTaintedUrlSuffix() } + + /** DEPRECATED. Gets the flow state corresponding to `label`. */ + deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } +} diff --git a/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll index 7da1503a830e..46fdc602d6d6 100644 --- a/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll @@ -12,56 +12,7 @@ private import semmle.javascript.dataflow.internal.DataFlowPrivate as DataFlowPr */ module TaintedUrlSuffix { private import DataFlow - - private newtype TFlowState = - TTaint() or - TTaintedUrlSuffix() - - /** - * A flow state with two values, `taint` and `tainted-url-suffix`. - * - * The `tainted-url-suffix` state represents a URL with a tainted query and fragment part, - * which we collectively refer to as the "suffix" of the URL. - * - * The `taint` state corresponds to ordinary taint. - */ - class FlowState extends TFlowState { - /** - * Holds if this represents a value that is considered entirely tainted. - */ - predicate isTaint() { this = TTaint() } - - /** - * Holds if this represents a URL whose fragment and/or query parts are considered tainted. - */ - predicate isTaintedUrlSuffix() { this = TTaintedUrlSuffix() } - - /** Gets a string representation of this flow state. */ - string toString() { - this.isTaint() and result = "taint" - or - this.isTaintedUrlSuffix() and result = "tainted-url-suffix" - } - - /** DEPRECATED. Gets the corresponding flow label. */ - deprecated DataFlow::FlowLabel toFlowLabel() { - this.isTaint() and result.isTaint() - or - this.isTaintedUrlSuffix() and result instanceof TaintedUrlSuffixLabel - } - } - - /** Convenience predicates for working with flow states. */ - module FlowState { - /** Gets the `taint` flow state. */ - FlowState taint() { result.isTaint() } - - /** Gets the `tainted-url-suffix` flow state. */ - FlowState taintedUrlSuffix() { result.isTaintedUrlSuffix() } - - /** DEPRECATED. Gets the flow state correpsonding to `label`. */ - deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } - } + import CommonFlowState /** * The flow label representing a URL with a tainted query and fragment part. diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll index 836a501f418d..1b987ea2679e 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll @@ -8,9 +8,7 @@ import javascript private import semmle.javascript.security.TaintedUrlSuffixCustomizations module ClientSideUrlRedirect { - class FlowState = TaintedUrlSuffix::FlowState; - - module FlowState = TaintedUrlSuffix::FlowState; + import semmle.javascript.security.CommonFlowState /** * A data flow source for unvalidated URL redirect vulnerabilities. diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectQuery.qll index 9a6cff968fb5..0fdffc5e01cf 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectQuery.qll @@ -21,7 +21,7 @@ deprecated private class ConcreteDocumentUrl extends DocumentUrl { * A taint-tracking configuration for reasoning about unvalidated URL redirections. */ module ClientSideUrlRedirectConfig implements DataFlow::StateConfigSig { - class FlowState = TaintedUrlSuffix::FlowState; + import semmle.javascript.security.CommonFlowState predicate isSource(DataFlow::Node source, FlowState state) { source.(Source).getAFlowState() = state From 12289d4c3993810ff2aeb499ef8e030bd4885141 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 11 Dec 2024 11:05:16 +0100 Subject: [PATCH 05/37] JS: Migrate DomBasedXssQuery to FlowState --- .../javascript/security/CommonFlowState.qll | 11 +++ .../dataflow/DomBasedXssCustomizations.qll | 15 ++-- .../security/dataflow/DomBasedXssQuery.qll | 75 ++++++++++--------- 3 files changed, 58 insertions(+), 43 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/CommonFlowState.qll b/javascript/ql/lib/semmle/javascript/security/CommonFlowState.qll index 1d4e8746312e..c49a053276ae 100644 --- a/javascript/ql/lib/semmle/javascript/security/CommonFlowState.qll +++ b/javascript/ql/lib/semmle/javascript/security/CommonFlowState.qll @@ -8,6 +8,7 @@ private import TaintedUrlSuffixCustomizations private newtype TFlowState = TTaint() or TTaintedUrlSuffix() or + TTaintedPrefix() /** * A flow state indicating which part of a value is tainted. @@ -24,6 +25,11 @@ class FlowState extends TFlowState { */ predicate isTaintedUrlSuffix() { this = TTaintedUrlSuffix() } + /** + * Holds if this represents a string whose prefix is known to be tainted. + */ + predicate isTaintedPrefix() { this = TTaintedPrefix() } + /** Gets a string representation of this flow state. */ string toString() { this.isTaint() and result = "taint" @@ -56,6 +62,11 @@ module FlowState { */ FlowState taintedUrlSuffix() { result.isTaintedUrlSuffix() } + /** + * Gets the flow state representing a string whose prefix is known to be tainted. + */ + FlowState taintedPrefix() { result.isTaintedPrefix() } + /** DEPRECATED. Gets the flow state corresponding to `label`. */ deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll index 8dd89667959d..731ef1eb721c 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll @@ -8,6 +8,7 @@ private import semmle.javascript.dataflow.InferredTypes module DomBasedXss { private import Xss::Shared as Shared + import semmle.javascript.security.CommonFlowState /** A data flow source for DOM-based XSS vulnerabilities. */ abstract class Source extends Shared::Source { } @@ -28,16 +29,16 @@ module DomBasedXss { predicate blocksExpr(boolean outcome, Expr e) { none() } /** - * Holds if this node acts as a barrier for `label`, blocking further flow from `e` if `this` evaluates to `outcome`. + * Holds if this node acts as a barrier for `state`, blocking further flow from `e` if `this` evaluates to `outcome`. */ - predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { none() } + predicate blocksExpr(boolean outcome, Expr e, FlowState state) { none() } /** DEPRECATED. Use `blocksExpr` instead. */ deprecated predicate sanitizes(boolean outcome, Expr e) { this.blocksExpr(outcome, e) } /** DEPRECATED. Use `blocksExpr` instead. */ deprecated predicate sanitizes(boolean outcome, Expr e, DataFlow::FlowLabel label) { - this.blocksExpr(outcome, e, label) + this.blocksExpr(outcome, e, FlowState::fromFlowLabel(label)) } } @@ -379,20 +380,20 @@ module DomBasedXss { /** * A flow-label representing tainted values where the prefix is attacker controlled. */ - abstract class PrefixString extends DataFlow::FlowLabel { + abstract deprecated class PrefixString extends DataFlow::FlowLabel { PrefixString() { this = "PrefixString" } } /** Gets the flow-label representing tainted values where the prefix is attacker controlled. */ - PrefixString prefixLabel() { any() } + deprecated PrefixString prefixLabel() { any() } /** * A sanitizer that blocks the `PrefixString` label when the start of the string is being tested as being of a particular prefix. */ abstract class PrefixStringSanitizer extends BarrierGuard instanceof StringOps::StartsWith { - override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { e = super.getBaseString().asExpr() and - label = prefixLabel() and + state.isTaintedPrefix() and outcome = super.getPolarity() } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssQuery.qll index d8e38456b8fc..6979ec12a2e7 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssQuery.qll @@ -27,81 +27,83 @@ class HtmlSink extends DataFlow::Node instanceof Sink { * - URL sinks are only sinks when the scheme is user controlled * - JQuery selector sinks are sinks when the tainted value can start with `<`. * - * The above is achieved using three flow labels: + * The above is achieved using three flow states: * - TaintedUrlSuffix: a URL where the attacker only controls a suffix. * - Taint: a tainted value where the attacker controls part of the value. * - PrefixLabel: a tainted value where the attacker controls the prefix */ module DomBasedXssConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + import semmle.javascript.security.CommonFlowState - predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { + predicate isSource(DataFlow::Node source, FlowState state) { source instanceof Source and - (label.isTaint() or label = prefixLabel()) and + (state.isTaint() or state.isTaintedPrefix()) and not source = TaintedUrlSuffix::source() or source = TaintedUrlSuffix::source() and - label = TaintedUrlSuffix::label() + state.isTaintedUrlSuffix() } - predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { + predicate isSink(DataFlow::Node sink, FlowState state) { sink instanceof HtmlSink and - label = [TaintedUrlSuffix::label(), prefixLabel(), DataFlow::FlowLabel::taint()] + (state.isTaint() or state.isTaintedPrefix() or state.isTaintedUrlSuffix()) or sink instanceof JQueryHtmlOrSelectorSink and - label = [DataFlow::FlowLabel::taint(), prefixLabel()] + (state.isTaint() or state.isTaintedPrefix()) or sink instanceof WriteUrlSink and - label = prefixLabel() + state.isTaintedPrefix() } predicate isBarrier(DataFlow::Node node) { - node instanceof Sanitizer or node = Shared::BarrierGuard::getABarrierNode() + node instanceof Sanitizer + or + node = Shared::BarrierGuard::getABarrierNode() + or + isOptionallySanitizedNode(node) } - predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel lbl) { - // copy all taint barrier guards to the TaintedUrlSuffix/PrefixLabel label + predicate isBarrier(DataFlow::Node node, FlowState state) { + // copy all taint barrier guards to the TaintedUrlSuffix/PrefixLabel state TaintTracking::defaultSanitizer(node) and - lbl = [TaintedUrlSuffix::label(), prefixLabel()] + (state.isTaintedUrlSuffix() or state.isTaintedPrefix()) or - // any non-first string-concatenation leaf is a barrier for the prefix label. + // any non-first string-concatenation leaf is a barrier for the prefix state. exists(StringOps::ConcatenationRoot root | node = root.getALeaf() and not node = root.getFirstLeaf() and - lbl = prefixLabel() + state.isTaintedPrefix() ) or - // we assume that `.join()` calls have a prefix, and thus block the prefix label. + // we assume that `.join()` calls have a prefix, and thus block the prefix state. node = any(DataFlow::MethodCallNode call | call.getMethodName() = "join") and - lbl = prefixLabel() + state.isTaintedPrefix() or - isOptionallySanitizedNode(node) and - lbl = [DataFlow::FlowLabel::taint(), prefixLabel(), TaintedUrlSuffix::label()] + TaintedUrlSuffix::isStateBarrier(node, TaintedUrlSuffix::FlowState::taintedUrlSuffix()) and + state.isTaintedUrlSuffix() or - TaintedUrlSuffix::isBarrier(node, lbl) - or - node = DataFlow::MakeLabeledBarrierGuard::getABarrierNode(lbl) + node = DataFlow::MakeStateBarrierGuard::getABarrierNode(state) } - predicate isBarrierIn(DataFlow::Node node, DataFlow::FlowLabel label) { isSource(node, label) } + predicate isBarrierIn(DataFlow::Node node, FlowState state) { isSource(node, state) } predicate isAdditionalFlowStep( - DataFlow::Node node1, DataFlow::FlowLabel state1, DataFlow::Node node2, - DataFlow::FlowLabel state2 + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - TaintedUrlSuffix::step(node1, node2, state1, state2) + TaintedUrlSuffix::isAdditionalFlowStep(node1, state1, node2, state2) or exists(DataFlow::Node operator | StringConcatenation::taintStep(node1, node2, operator, _) and StringConcatenation::getOperand(operator, 0).getStringValue() = "<" + any(string s) and - state1 = TaintedUrlSuffix::label() and + state1.isTaintedUrlSuffix() and state2.isTaint() ) or - // steps out of taintedSuffixlabel to taint-label are also steps to prefixLabel. - TaintedUrlSuffix::step(node1, node2, TaintedUrlSuffix::label(), DataFlow::FlowLabel::taint()) and - state1 = TaintedUrlSuffix::label() and - state2 = prefixLabel() + // steps out of tainted-url-suffix to taint are also steps to tainted-prefix. + TaintedUrlSuffix::isAdditionalFlowStep(node1, FlowState::taintedUrlSuffix(), node2, + FlowState::taint()) and + state1.isTaintedUrlSuffix() and + state2.isTaintedPrefix() or // FIXME: this fails to work in the test case at jquery.js:37 exists(DataFlow::FunctionNode callback, DataFlow::Node arg | @@ -126,24 +128,25 @@ deprecated class Configuration extends TaintTracking::Configuration { Configuration() { this = "HtmlInjection" } override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { - DomBasedXssConfig::isSource(source, label) + DomBasedXssConfig::isSource(source, FlowState::fromFlowLabel(label)) } override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - DomBasedXssConfig::isSink(sink, label) + DomBasedXssConfig::isSink(sink, FlowState::fromFlowLabel(label)) } override predicate isSanitizer(DataFlow::Node node) { DomBasedXssConfig::isBarrier(node) } override predicate isLabeledBarrier(DataFlow::Node node, DataFlow::FlowLabel lbl) { - DomBasedXssConfig::isBarrier(node, lbl) + DomBasedXssConfig::isBarrier(node, FlowState::fromFlowLabel(lbl)) } override predicate isAdditionalFlowStep( DataFlow::Node node1, DataFlow::Node node2, DataFlow::FlowLabel state1, DataFlow::FlowLabel state2 ) { - DomBasedXssConfig::isAdditionalFlowStep(node1, state1, node2, state2) + DomBasedXssConfig::isAdditionalFlowStep(node1, FlowState::fromFlowLabel(state1), node2, + FlowState::fromFlowLabel(state2)) or // inherit all ordinary taint steps for the prefix label state1 = prefixLabel() and @@ -156,7 +159,7 @@ private class PrefixStringSanitizerActivated extends PrefixStringSanitizer { PrefixStringSanitizerActivated() { this = this } } -private class PrefixStringActivated extends DataFlow::FlowLabel, PrefixString { +deprecated private class PrefixStringActivated extends DataFlow::FlowLabel, PrefixString { PrefixStringActivated() { this = this } } From 14ca1c134b28d03152fa892149ac469c39041ae6 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 11 Dec 2024 14:40:32 +0100 Subject: [PATCH 06/37] JS: Update TaintedUrlSuffix test --- .../library-tests/TaintedUrlSuffix/test.ql | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/javascript/ql/test/library-tests/TaintedUrlSuffix/test.ql b/javascript/ql/test/library-tests/TaintedUrlSuffix/test.ql index f3d43dd41047..3c247a4e12c2 100644 --- a/javascript/ql/test/library-tests/TaintedUrlSuffix/test.ql +++ b/javascript/ql/test/library-tests/TaintedUrlSuffix/test.ql @@ -3,17 +3,17 @@ import testUtilities.InlineExpectationsTest import semmle.javascript.security.TaintedUrlSuffix module TestConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + import semmle.javascript.security.CommonFlowState - predicate isSource(DataFlow::Node node, DataFlow::FlowLabel state) { - node = TaintedUrlSuffix::source() and state = TaintedUrlSuffix::label() + predicate isSource(DataFlow::Node node, FlowState state) { + node = TaintedUrlSuffix::source() and state.isTaintedUrlSuffix() or node instanceof RemoteFlowSource and not node = TaintedUrlSuffix::source() and state.isTaint() } - predicate isSink(DataFlow::Node node, DataFlow::FlowLabel state) { none() } + predicate isSink(DataFlow::Node node, FlowState state) { none() } predicate isSink(DataFlow::Node node) { exists(DataFlow::CallNode call | @@ -23,14 +23,13 @@ module TestConfig implements DataFlow::StateConfigSig { } predicate isAdditionalFlowStep( - DataFlow::Node node1, DataFlow::FlowLabel state1, DataFlow::Node node2, - DataFlow::FlowLabel state2 + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - TaintedUrlSuffix::step(node1, node2, state1, state2) + TaintedUrlSuffix::isAdditionalFlowStep(node1, state1, node2, state2) } - predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel label) { - TaintedUrlSuffix::isBarrier(node, label) + predicate isBarrier(DataFlow::Node node, FlowState state) { + TaintedUrlSuffix::isStateBarrier(node, state) } } @@ -44,7 +43,7 @@ module InlineTest implements TestSig { exists(TestFlow::PathNode src, TestFlow::PathNode sink | TestFlow::flowPath(src, sink) | sink.getLocation() = location and element = "" and - value = sink.getState() + value = sink.getState().toString() ) } } From 5f42a715f6abc6f5e5bae5329f1589e1390a644a Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 11 Dec 2024 15:14:47 +0100 Subject: [PATCH 07/37] JS: Migrate TaintedObject to a CommonFlowState --- .../javascript/security/CommonFlowState.qll | 20 ++++++- .../javascript/security/TaintedObject.qll | 60 +++++++++++-------- .../security/TaintedObjectCustomizations.qll | 6 +- 3 files changed, 57 insertions(+), 29 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/CommonFlowState.qll b/javascript/ql/lib/semmle/javascript/security/CommonFlowState.qll index c49a053276ae..52e1e0d00f38 100644 --- a/javascript/ql/lib/semmle/javascript/security/CommonFlowState.qll +++ b/javascript/ql/lib/semmle/javascript/security/CommonFlowState.qll @@ -4,11 +4,13 @@ private import javascript private import TaintedUrlSuffixCustomizations +private import TaintedObjectCustomizations private newtype TFlowState = TTaint() or TTaintedUrlSuffix() or - TTaintedPrefix() + TTaintedPrefix() or + TTaintedObject() /** * A flow state indicating which part of a value is tainted. @@ -30,6 +32,12 @@ class FlowState extends TFlowState { */ predicate isTaintedPrefix() { this = TTaintedPrefix() } + /** + * Holds if this represents a deeply tainted object, such as a JSON object + * parsed from user-controlled data. + */ + predicate isTaintedObject() { this = TTaintedObject() } + /** Gets a string representation of this flow state. */ string toString() { this.isTaint() and result = "taint" @@ -37,6 +45,8 @@ class FlowState extends TFlowState { this.isTaintedUrlSuffix() and result = "tainted-url-suffix" or this.isTaintedPrefix() and result = "tainted-prefix" + or + this.isTaintedObject() and result = "tainted-object" } /** DEPRECATED. Gets the corresponding flow label. */ @@ -46,6 +56,8 @@ class FlowState extends TFlowState { this.isTaintedUrlSuffix() and result = TaintedUrlSuffix::label() or this.isTaintedPrefix() and result = "PrefixString" + or + this.isTaintedObject() and result = TaintedObject::label() } } @@ -67,6 +79,12 @@ module FlowState { */ FlowState taintedPrefix() { result.isTaintedPrefix() } + /** + * Gets the flow state representing a deeply tainted object, such as a JSON object + * parsed from user-controlled data. + */ + FlowState taintedObject() { result.isTaintedObject() } + /** DEPRECATED. Gets the flow state corresponding to `label`. */ deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } } diff --git a/javascript/ql/lib/semmle/javascript/security/TaintedObject.qll b/javascript/ql/lib/semmle/javascript/security/TaintedObject.qll index 4b230d0de669..97cef346ce48 100644 --- a/javascript/ql/lib/semmle/javascript/security/TaintedObject.qll +++ b/javascript/ql/lib/semmle/javascript/security/TaintedObject.qll @@ -7,10 +7,10 @@ * * To track deeply tainted objects, a flow-tracking configuration should generally include the following: * - * 1. One or more sinks associated with the label `TaintedObject::label()`. - * 2. The sources from `TaintedObject::isSource`. - * 3. The flow steps from `TaintedObject::step`. - * 4. The sanitizing guards `TaintedObject::SanitizerGuard`. + * 1. One or more sinks associated with the flow state `FlowState::taintedObject()`. + * 2. The sources from `TaintedObject::Source`. + * 3. The flow steps from `TaintedObject::isAdditionalFlowStep`. + * 4. The barriers from `TaintedObject::SanitizerGuard::getABarrierNode(state)`. */ import javascript @@ -22,35 +22,39 @@ module TaintedObject { import TaintedObjectCustomizations::TaintedObject // Materialize flow labels - private class ConcreteTaintedObjectLabel extends TaintedObjectLabel { + deprecated private class ConcreteTaintedObjectLabel extends TaintedObjectLabel { ConcreteTaintedObjectLabel() { this = this } } + deprecated predicate step(Node src, Node trg, FlowLabel inlbl, FlowLabel outlbl) { + isAdditionalFlowStep(src, FlowState::fromFlowLabel(inlbl), trg, FlowState::fromFlowLabel(outlbl)) + } + /** * Holds for the flows steps that are relevant for tracking user-controlled JSON objects. */ - predicate step(Node src, Node trg, FlowLabel inlbl, FlowLabel outlbl) { + predicate isAdditionalFlowStep(Node src, FlowState inlbl, Node trg, FlowState outlbl) { // JSON parsers map tainted inputs to tainted JSON - inlbl.isDataOrTaint() and - outlbl = label() and + inlbl.isTaint() and + outlbl.isTaintedObject() and exists(JsonParserCall parse | src = parse.getInput() and trg = parse.getOutput() ) or // Property reads preserve deep object taint. - inlbl = label() and - outlbl = label() and + inlbl.isTaintedObject() and + outlbl.isTaintedObject() and trg.(PropRead).getBase() = src or // Property projection preserves deep object taint - inlbl = label() and - outlbl = label() and + inlbl.isTaintedObject() and + outlbl.isTaintedObject() and trg.(PropertyProjection).getObject() = src or // Extending objects preserves deep object taint - inlbl = label() and - outlbl = label() and + inlbl.isTaintedObject() and + outlbl.isTaintedObject() and exists(ExtendCall call | src = call.getAnOperand() and trg = call @@ -60,8 +64,8 @@ module TaintedObject { ) or // Spreading into an object preserves deep object taint: `p -> { ...p }` - inlbl = label() and - outlbl = label() and + inlbl.isTaintedObject() and + outlbl.isTaintedObject() and exists(ObjectLiteralNode obj | src = obj.getASpreadProperty() and trg = obj @@ -69,9 +73,13 @@ module TaintedObject { } /** + * DEPRECATED. Use the `Source` class and `FlowState#isTaintedObject()` directly. + * * Holds if `node` is a source of JSON taint and label is the JSON taint label. */ - predicate isSource(Node source, FlowLabel label) { source instanceof Source and label = label() } + deprecated predicate isSource(Node source, FlowLabel label) { + source instanceof Source and label = label() + } /** Request input accesses as a JSON source. */ private class RequestInputAsSource extends Source { @@ -86,11 +94,11 @@ module TaintedObject { predicate blocksExpr(boolean outcome, Expr e) { none() } /** Holds if this node blocks flow of `label` through `e`, provided it evaluates to `outcome`. */ - predicate blocksExpr(boolean outcome, Expr e, FlowLabel label) { none() } + predicate blocksExpr(boolean outcome, Expr e, FlowState label) { none() } /** DEPRECATED. Use `blocksExpr` instead. */ deprecated predicate sanitizes(boolean outcome, Expr e, FlowLabel label) { - this.blocksExpr(outcome, e, label) + this.blocksExpr(outcome, e, FlowState::fromFlowLabel(label)) } /** DEPRECATED. Use `blocksExpr` instead. */ @@ -111,7 +119,7 @@ module TaintedObject { /** * A sanitizer guard that blocks deep object taint. */ - module SanitizerGuard = DataFlow::MakeLabeledBarrierGuard; + module SanitizerGuard = DataFlow::MakeStateBarrierGuard; /** * A test of form `typeof x === "something"`, preventing `x` from being an object in some cases. @@ -133,10 +141,10 @@ module TaintedObject { ) } - override predicate blocksExpr(boolean outcome, Expr e, FlowLabel label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { polarity = outcome and e = operand and - label = label() + state.isTaintedObject() } } @@ -161,8 +169,8 @@ module TaintedObject { .getACall() } - override predicate blocksExpr(boolean outcome, Expr e, FlowLabel lbl) { - e = super.getAnArgument().asExpr() and outcome = true and lbl = label() + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { + e = super.getAnArgument().asExpr() and outcome = true and state.isTaintedObject() } } @@ -175,10 +183,10 @@ module TaintedObject { JsonSchemaValidationGuard() { this = call.getAValidationResultAccess(polarity) } - override predicate blocksExpr(boolean outcome, Expr e, FlowLabel label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { outcome = polarity and e = call.getInput().asExpr() and - label = label() + state.isTaintedObject() } } } diff --git a/javascript/ql/lib/semmle/javascript/security/TaintedObjectCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/TaintedObjectCustomizations.qll index 8e0fd38f15a4..5dc687deecae 100644 --- a/javascript/ql/lib/semmle/javascript/security/TaintedObjectCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/TaintedObjectCustomizations.qll @@ -7,8 +7,10 @@ import javascript /** Provides classes and predicates for reasoning about deeply tainted objects. */ module TaintedObject { + import CommonFlowState + /** A flow label representing a deeply tainted object. */ - abstract class TaintedObjectLabel extends DataFlow::FlowLabel { + abstract deprecated class TaintedObjectLabel extends DataFlow::FlowLabel { TaintedObjectLabel() { this = "tainted-object" } } @@ -19,7 +21,7 @@ module TaintedObject { * * Note that the presence of the this label generally implies the presence of the `taint` label as well. */ - DataFlow::FlowLabel label() { result instanceof TaintedObjectLabel } + deprecated DataFlow::FlowLabel label() { result instanceof TaintedObjectLabel } /** * A source of a user-controlled deep object. From 15d999a9dc30904b305a3ecdb0b8580ddb7e41f3 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 11 Dec 2024 15:15:02 +0100 Subject: [PATCH 08/37] JS: Migrate DeepObjectResourceExhaustion --- ...pObjectResourceExhaustionCustomizations.qll | 13 +++++++++---- .../DeepObjectResourceExhaustionQuery.qll | 18 +++++++++--------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/DeepObjectResourceExhaustionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/DeepObjectResourceExhaustionCustomizations.qll index 58d8d02808ef..eaac474b2074 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/DeepObjectResourceExhaustionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/DeepObjectResourceExhaustionCustomizations.qll @@ -11,21 +11,26 @@ private import semmle.javascript.security.TaintedObjectCustomizations * DoS attacks due to inefficient handling of user-controlled objects. */ module DeepObjectResourceExhaustion { + import semmle.javascript.security.CommonFlowState + /** * A data flow source for inefficient handling of user-controlled objects. */ abstract class Source extends DataFlow::Node { - /** Gets a flow label to associate with this source. */ - DataFlow::FlowLabel getAFlowLabel() { result = TaintedObject::label() } + /** Gets a flow state to associate with this source. */ + FlowState getAFlowState() { result.isTaintedObject() } + + /** DEPRECATED. Use `getAFlowState()` instead. */ + deprecated DataFlow::FlowLabel getAFlowLabel() { result = this.getAFlowState().toFlowLabel() } } private class TaintedObjectSourceAsSource extends Source instanceof TaintedObject::Source { - override DataFlow::FlowLabel getAFlowLabel() { result = TaintedObject::label() } + override FlowState getAFlowState() { result.isTaintedObject() } } /** An active threat-model source, considered as a flow source. */ private class ActiveThreatModelSourceAsSource extends Source, ActiveThreatModelSource { - override DataFlow::FlowLabel getAFlowLabel() { result.isTaint() } + override FlowState getAFlowState() { result.isTaint() } } /** diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/DeepObjectResourceExhaustionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/DeepObjectResourceExhaustionQuery.qll index 84053319d021..ca40447145c5 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/DeepObjectResourceExhaustionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/DeepObjectResourceExhaustionQuery.qll @@ -12,26 +12,26 @@ import DeepObjectResourceExhaustionCustomizations::DeepObjectResourceExhaustion * of user-controlled objects. */ module DeepObjectResourceExhaustionConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + import semmle.javascript.security.CommonFlowState - predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { - source.(Source).getAFlowLabel() = label + predicate isSource(DataFlow::Node source, FlowState state) { + source.(Source).getAFlowState() = state } - predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - sink instanceof Sink and label = TaintedObject::label() + predicate isSink(DataFlow::Node sink, FlowState state) { + sink instanceof Sink and state.isTaintedObject() } - predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel label) { - node = TaintedObject::SanitizerGuard::getABarrierNode(label) + predicate isBarrier(DataFlow::Node node, FlowState state) { + node = TaintedObject::SanitizerGuard::getABarrierNode(state) } predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } predicate isAdditionalFlowStep( - DataFlow::Node src, DataFlow::FlowLabel inlbl, DataFlow::Node trg, DataFlow::FlowLabel outlbl + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - TaintedObject::step(src, trg, inlbl, outlbl) + TaintedObject::isAdditionalFlowStep(node1, state1, node2, state2) } } From daddff0dc66b5865e3fef51ecf749e9ae18d4a7e Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 11 Dec 2024 15:16:03 +0100 Subject: [PATCH 09/37] JS: Avoid deprecation warning in XssThroughDom --- .../semmle/javascript/security/dataflow/XssThroughDomQuery.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomQuery.qll index 086cbe1abf06..7d6564fce10c 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomQuery.qll @@ -106,7 +106,7 @@ private class PrefixStringSanitizer extends DomBasedXss::PrefixStringSanitizer { PrefixStringSanitizer() { this = this } } -private class PrefixString extends DataFlow::FlowLabel, DomBasedXss::PrefixString { +deprecated private class PrefixString extends DataFlow::FlowLabel, DomBasedXss::PrefixString { PrefixString() { this = this } } From 8e8de5cf23d3034d387308bbd280fa5a4256e194 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 12 Dec 2024 13:08:00 +0100 Subject: [PATCH 10/37] JS: Migrate LoopBoundInjection --- .../LoopBoundInjectionCustomizations.qll | 19 +++++++++--------- .../dataflow/LoopBoundInjectionQuery.qll | 20 +++++++++---------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/LoopBoundInjectionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/LoopBoundInjectionCustomizations.qll index f018428252c8..63da6fca1054 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/LoopBoundInjectionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/LoopBoundInjectionCustomizations.qll @@ -8,6 +8,7 @@ import javascript module LoopBoundInjection { import semmle.javascript.security.TaintedObject + import semmle.javascript.security.CommonFlowState /** * Holds if an exception will be thrown whenever `e` evaluates to `undefined` or `null`. @@ -176,16 +177,16 @@ module LoopBoundInjection { predicate blocksExpr(boolean outcome, Expr e) { none() } /** - * Holds if this node acts as a barrier for `label`, blocking further flow from `e` if `this` evaluates to `outcome`. + * Holds if this node acts as a barrier for `state`, blocking further flow from `e` if `this` evaluates to `outcome`. */ - predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { none() } + predicate blocksExpr(boolean outcome, Expr e, FlowState state) { none() } /** DEPRECATED. Use `blocksExpr` instead. */ deprecated predicate sanitizes(boolean outcome, Expr e) { this.blocksExpr(outcome, e) } /** DEPRECATED. Use `blocksExpr` instead. */ deprecated predicate sanitizes(boolean outcome, Expr e, DataFlow::FlowLabel label) { - this.blocksExpr(outcome, e, label) + this.blocksExpr(outcome, e, FlowState::fromFlowLabel(label)) } } @@ -214,10 +215,10 @@ module LoopBoundInjection { IsArraySanitizerGuard() { astNode.getCalleeName() = "isArray" } - override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { true = outcome and e = astNode.getAnArgument() and - label = TaintedObject::label() + state.isTaintedObject() } } @@ -232,10 +233,10 @@ module LoopBoundInjection { DataFlow::globalVarRef("Array").flowsToExpr(astNode.getRightOperand()) } - override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { true = outcome and e = astNode.getLeftOperand() and - label = TaintedObject::label() + state.isTaintedObject() } } @@ -253,10 +254,10 @@ module LoopBoundInjection { propRead.getPropertyName() = "length" } - override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { false = outcome and e = propRead.getBase().asExpr() and - label = TaintedObject::label() + state.isTaintedObject() } } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/LoopBoundInjectionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/LoopBoundInjectionQuery.qll index 0c4d0e52004f..b4b799b06da9 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/LoopBoundInjectionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/LoopBoundInjectionQuery.qll @@ -14,29 +14,29 @@ import LoopBoundInjectionCustomizations::LoopBoundInjection * A taint tracking configuration for reasoning about looping on tainted objects with unbounded length. */ module LoopBoundInjectionConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + import semmle.javascript.security.CommonFlowState - predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { - source instanceof Source and label = TaintedObject::label() + predicate isSource(DataFlow::Node source, FlowState state) { + source instanceof Source and state.isTaintedObject() } - predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - sink instanceof Sink and label = TaintedObject::label() + predicate isSink(DataFlow::Node sink, FlowState state) { + sink instanceof Sink and state.isTaintedObject() } predicate isBarrier(DataFlow::Node node) { node = DataFlow::MakeBarrierGuard::getABarrierNode() } - predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel label) { - node = DataFlow::MakeLabeledBarrierGuard::getABarrierNode(label) or - node = TaintedObject::SanitizerGuard::getABarrierNode(label) + predicate isBarrier(DataFlow::Node node, FlowState state) { + node = DataFlow::MakeStateBarrierGuard::getABarrierNode(state) or + node = TaintedObject::SanitizerGuard::getABarrierNode(state) } predicate isAdditionalFlowStep( - DataFlow::Node src, DataFlow::FlowLabel inlbl, DataFlow::Node trg, DataFlow::FlowLabel outlbl + DataFlow::Node src, FlowState inlbl, DataFlow::Node trg, FlowState outlbl ) { - TaintedObject::step(src, trg, inlbl, outlbl) + TaintedObject::isAdditionalFlowStep(src, inlbl, trg, outlbl) } } From c38e3a23eb79f850f97437e64f24d24ecdd1dc45 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 12 Dec 2024 13:19:00 +0100 Subject: [PATCH 11/37] JS: Migrate NoSqlInjection --- .../dataflow/NosqlInjectionCustomizations.qll | 7 +++++- .../security/dataflow/NosqlInjectionQuery.qll | 24 +++++++++---------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/NosqlInjectionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/NosqlInjectionCustomizations.qll index 536276d5c1dc..36c0601d501c 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/NosqlInjectionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/NosqlInjectionCustomizations.qll @@ -8,6 +8,8 @@ import javascript import semmle.javascript.security.TaintedObject module NosqlInjection { + import semmle.javascript.security.CommonFlowState + /** * A data flow source for NoSQL injection vulnerabilities. */ @@ -22,7 +24,10 @@ module NosqlInjection { * * Defaults to deeply tainted objects only. */ - DataFlow::FlowLabel getAFlowLabel() { result = TaintedObject::label() } + FlowState getAFlowState() { result.isTaintedObject() } + + /** DEPRECATED. Use `getAFlowState()` instead. */ + deprecated DataFlow::FlowLabel getAFlowLabel() { result = this.getAFlowState().toFlowLabel() } } /** diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/NosqlInjectionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/NosqlInjectionQuery.qll index a213fa5aa4a1..dbb5140d7c42 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/NosqlInjectionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/NosqlInjectionQuery.qll @@ -15,19 +15,17 @@ import NosqlInjectionCustomizations::NosqlInjection * A taint-tracking configuration for reasoning about SQL-injection vulnerabilities. */ module NosqlInjectionConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + import semmle.javascript.security.CommonFlowState - predicate isSource(DataFlow::Node source, DataFlow::FlowLabel state) { + predicate isSource(DataFlow::Node source, FlowState state) { source instanceof Source and state.isTaint() or - TaintedObject::isSource(source, state) + source instanceof TaintedObject::Source and state.isTaintedObject() } - predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel state) { - sink.(Sink).getAFlowLabel() = state - } + predicate isSink(DataFlow::Node sink, FlowState state) { sink.(Sink).getAFlowState() = state } - predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel state) { + predicate isBarrier(DataFlow::Node node, FlowState state) { node instanceof Sanitizer and state.isTaint() or TaintTracking::defaultSanitizer(node) and state.isTaint() @@ -36,14 +34,13 @@ module NosqlInjectionConfig implements DataFlow::StateConfigSig { } predicate isAdditionalFlowStep( - DataFlow::Node node1, DataFlow::FlowLabel state1, DataFlow::Node node2, - DataFlow::FlowLabel state2 + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - TaintedObject::step(node1, node2, state1, state2) + TaintedObject::isAdditionalFlowStep(node1, state1, node2, state2) or // additional flow step to track taint through NoSQL query objects - state1 = TaintedObject::label() and - state2 = TaintedObject::label() and + state1.isTaintedObject() and + state2.isTaintedObject() and exists(NoSql::Query query, DataFlow::SourceNode queryObj | queryObj.flowsTo(query) and queryObj.flowsTo(node2) and @@ -90,6 +87,7 @@ deprecated class Configuration extends TaintTracking::Configuration { DataFlow::Node node1, DataFlow::Node node2, DataFlow::FlowLabel state1, DataFlow::FlowLabel state2 ) { - NosqlInjectionConfig::isAdditionalFlowStep(node1, state1, node2, state2) + NosqlInjectionConfig::isAdditionalFlowStep(node1, FlowState::fromFlowLabel(state1), node2, + FlowState::fromFlowLabel(state2)) } } From 355f7cdd5407ee17bef589a8c8b756ce064eb5bb Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 12 Dec 2024 13:36:28 +0100 Subject: [PATCH 12/37] JS: Migrate PrototypePollutingMergeCall --- .../PrototypePollutionCustomizations.qll | 31 +++++++++++-------- .../dataflow/PrototypePollutionQuery.qll | 20 +++++------- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionCustomizations.qll index 0426b413b445..1e95e4b550fb 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionCustomizations.qll @@ -9,7 +9,12 @@ import semmle.javascript.security.TaintedObject import semmle.javascript.dependencies.SemVer module PrototypePollution { + import semmle.javascript.security.CommonFlowState + /** + * DEPRECATED. This flow label is no longer in use, and there is no corresponding flow state, as + * the query instead relies on implicit reads at the sinks. + * * A label for wrappers around tainted objects, that is, objects that are * not completely user-controlled, but contain a user-controlled object. * @@ -23,12 +28,12 @@ module PrototypePollution { * } * ``` */ - abstract class TaintedObjectWrapper extends DataFlow::FlowLabel { + abstract deprecated class TaintedObjectWrapper extends DataFlow::FlowLabel { TaintedObjectWrapper() { this = "tainted-object-wrapper" } } - /** Companion module to the `TaintedObjectWrapper` class. */ - module TaintedObjectWrapper { + /** DEPRECATED. Use `FlowState::taintedObjectWrapper()` instead. */ + deprecated module TaintedObjectWrapper { /** Gets the instance of the `TaintedObjectWrapper` label. */ TaintedObjectWrapper label() { any() } } @@ -40,7 +45,10 @@ module PrototypePollution { /** * Gets the type of data coming from this source. */ - abstract DataFlow::FlowLabel getAFlowLabel(); + FlowState getAFlowState() { result.isTaintedObject() } + + /** DEPRECATED. Use `getAFlowState()` instead. */ + deprecated DataFlow::FlowLabel getAFlowLabel() { result = this.getAFlowState().toFlowLabel() } } /** @@ -50,7 +58,10 @@ module PrototypePollution { /** * Gets the type of data that can taint this sink. */ - abstract DataFlow::FlowLabel getAFlowLabel(); + FlowState getAFlowState() { result.isTaintedObject() } + + /** DEPRECATED. Use `getAFlowState()` instead. */ + deprecated DataFlow::FlowLabel getAFlowLabel() { result = this.getAFlowState().toFlowLabel() } /** * Holds if `moduleName` is the name of the module that defines this sink, @@ -68,14 +79,14 @@ module PrototypePollution { * in order to be flagged for prototype pollution. */ private class RemoteFlowAsSource extends Source instanceof RemoteFlowSource { - override DataFlow::FlowLabel getAFlowLabel() { result.isTaint() } + override FlowState getAFlowState() { result.isTaint() } } /** * A source of user-controlled objects. */ private class TaintedObjectSource extends Source instanceof TaintedObject::Source { - override DataFlow::FlowLabel getAFlowLabel() { result = TaintedObject::label() } + override FlowState getAFlowState() { result.isTaintedObject() } } class DeepExtendSink extends Sink { @@ -98,12 +109,6 @@ module PrototypePollution { ) } - override DataFlow::FlowLabel getAFlowLabel() { - result = TaintedObject::label() - or - result = TaintedObjectWrapper::label() - } - override predicate dependencyInfo(string moduleName_, Locatable loc) { moduleName = moduleName_ and location = loc diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionQuery.qll index 95020e12ec38..f5d54396ca7c 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionQuery.qll @@ -25,31 +25,27 @@ deprecated private class ConcreteTaintedObjectWrapper extends TaintedObjectWrapp * leading to prototype pollution. */ module PrototypePollutionConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + import semmle.javascript.security.CommonFlowState - predicate isSource(DataFlow::Node node, DataFlow::FlowLabel label) { - node.(Source).getAFlowLabel() = label - } + predicate isSource(DataFlow::Node node, FlowState state) { node.(Source).getAFlowState() = state } - predicate isSink(DataFlow::Node node, DataFlow::FlowLabel label) { - node.(Sink).getAFlowLabel() = label - } + predicate isSink(DataFlow::Node node, FlowState state) { node.(Sink).getAFlowState() = state } predicate isAdditionalFlowStep( - DataFlow::Node src, DataFlow::FlowLabel inlbl, DataFlow::Node dst, DataFlow::FlowLabel outlbl + DataFlow::Node src, FlowState inlbl, DataFlow::Node dst, FlowState outlbl ) { - TaintedObject::step(src, dst, inlbl, outlbl) + TaintedObject::isAdditionalFlowStep(src, inlbl, dst, outlbl) } predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet contents) { // For recursive merge sinks, the deeply tainted object only needs to be reachable from the input, the input itself // does not need to be deeply tainted. - isSink(node, TaintedObject::label()) and + isSink(node, FlowState::taintedObject()) and contents = DataFlow::ContentSet::anyProperty() } - predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel label) { - node = TaintedObject::SanitizerGuard::getABarrierNode(label) + predicate isBarrier(DataFlow::Node node, FlowState state) { + node = TaintedObject::SanitizerGuard::getABarrierNode(state) } } From 3573f0b065662a0ca36a77886580e732c1c5ccdb Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 12 Dec 2024 13:41:52 +0100 Subject: [PATCH 13/37] JS: Migrate SecondOrderCommandInjection --- ...ondOrderCommandInjectionCustomizations.qll | 32 ++++++++++++------- .../SecondOrderCommandInjectionQuery.qll | 22 ++++++------- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/SecondOrderCommandInjectionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/SecondOrderCommandInjectionCustomizations.qll index 520cf57b3af6..416ad56bef16 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/SecondOrderCommandInjectionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/SecondOrderCommandInjectionCustomizations.qll @@ -10,6 +10,8 @@ private import semmle.javascript.security.TaintedObjectCustomizations /** Classes and predicates for reasoning about second order command injection. */ module SecondOrderCommandInjection { + import semmle.javascript.security.CommonFlowState + /** A shell command that allows for second order command injection. */ private class VulnerableCommand extends string { VulnerableCommand() { this = ["git", "hg"] } @@ -39,8 +41,11 @@ module SecondOrderCommandInjection { /** Gets a string that describes the source. For use in the alert message. */ abstract string describe(); - /** Gets a label for which this is a source. */ - abstract DataFlow::FlowLabel getALabel(); + /** Gets a flow state for which this is a source. */ + FlowState getAFlowState() { result.isTaint() } + + /** DEPRECATED. Use `getAFlowState()` instead */ + deprecated DataFlow::FlowLabel getALabel() { result = this.getAFlowState().toFlowLabel() } } /** A parameter of an exported function, seen as a source for second order command injection. */ @@ -49,18 +54,18 @@ module SecondOrderCommandInjection { override string describe() { result = "library input" } - override DataFlow::FlowLabel getALabel() { result = TaintedObject::label() or result.isTaint() } + override FlowState getAFlowState() { result.isTaintedObject() or result.isTaint() } } /** A source of remote flow, seen as a source for second order command injection. */ class RemoteFlowAsSource extends Source instanceof RemoteFlowSource { override string describe() { result = "a user-provided value" } - override DataFlow::FlowLabel getALabel() { result.isTaint() } + override FlowState getAFlowState() { result.isTaint() } } private class TaintedObjectSourceAsSource extends Source instanceof TaintedObject::Source { - override DataFlow::FlowLabel getALabel() { result = TaintedObject::label() } + override FlowState getAFlowState() { result.isTaintedObject() } override string describe() { result = "a user-provided value" } } @@ -70,8 +75,11 @@ module SecondOrderCommandInjection { /** A sink for second order command injection. */ abstract class Sink extends DataFlow::Node { - /** Gets a label for which this is a sink. */ - abstract DataFlow::FlowLabel getALabel(); + /** Gets a flow state for which this is a sink. */ + FlowState getAFlowState() { result.isTaint() or result.isTaintedObject() } + + /** DERECATED. Use `getAFlowState()` instead. */ + deprecated DataFlow::FlowLabel getALabel() { result = this.getAFlowState().toFlowLabel() } /** Gets the command getting invoked. I.e. `git` or `hg`. */ abstract string getCommand(); @@ -93,16 +101,16 @@ module SecondOrderCommandInjection { predicate blocksExpr(boolean outcome, Expr e) { none() } /** - * Holds if this node acts as a barrier for `label`, blocking further flow from `e` if `this` evaluates to `outcome`. + * Holds if this node acts as a barrier for `state`, blocking further flow from `e` if `this` evaluates to `outcome`. */ - predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { none() } + predicate blocksExpr(boolean outcome, Expr e, FlowState state) { none() } /** DEPRECATED. Use `blocksExpr` instead. */ deprecated predicate sanitizes(boolean outcome, Expr e) { this.blocksExpr(outcome, e) } /** DEPRECATED. Use `blocksExpr` instead. */ deprecated predicate sanitizes(boolean outcome, Expr e, DataFlow::FlowLabel label) { - this.blocksExpr(outcome, e, label) + this.blocksExpr(outcome, e, FlowState::fromFlowLabel(label)) } } @@ -205,7 +213,7 @@ module SecondOrderCommandInjection { ) } - override DataFlow::FlowLabel getALabel() { result.isTaint() } + override FlowState getAFlowState() { result.isTaint() } } /** @@ -219,7 +227,7 @@ module SecondOrderCommandInjection { } // only vulnerable if an attacker controls the entire array - override DataFlow::FlowLabel getALabel() { result = TaintedObject::label() } + override FlowState getAFlowState() { result.isTaintedObject() } } /** diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/SecondOrderCommandInjectionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/SecondOrderCommandInjectionQuery.qll index 1fab45843a94..2de01d29b61f 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/SecondOrderCommandInjectionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/SecondOrderCommandInjectionQuery.qll @@ -15,33 +15,31 @@ private import semmle.javascript.security.TaintedObject * A taint-tracking configuration for reasoning about second order command-injection vulnerabilities. */ module SecondOrderCommandInjectionConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + import semmle.javascript.security.CommonFlowState - predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { - source.(Source).getALabel() = label + predicate isSource(DataFlow::Node source, FlowState state) { + source.(Source).getAFlowState() = state } - predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - sink.(Sink).getALabel() = label - } + predicate isSink(DataFlow::Node sink, FlowState state) { sink.(Sink).getAFlowState() = state } predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer or node = DataFlow::MakeBarrierGuard::getABarrierNode() } - predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel label) { + predicate isBarrier(DataFlow::Node node, FlowState state) { TaintTracking::defaultSanitizer(node) and - label.isTaint() + state.isTaint() or - node = DataFlow::MakeLabeledBarrierGuard::getABarrierNode(label) + node = DataFlow::MakeStateBarrierGuard::getABarrierNode(state) or - node = TaintedObject::SanitizerGuard::getABarrierNode(label) + node = TaintedObject::SanitizerGuard::getABarrierNode(state) } predicate isAdditionalFlowStep( - DataFlow::Node src, DataFlow::FlowLabel inlbl, DataFlow::Node trg, DataFlow::FlowLabel outlbl + DataFlow::Node src, FlowState inlbl, DataFlow::Node trg, FlowState outlbl ) { - TaintedObject::step(src, trg, inlbl, outlbl) + TaintedObject::isAdditionalFlowStep(src, inlbl, trg, outlbl) or // We're not using a taint-tracking config because taint steps would then apply to all flow states. // So we use a plain data flow config and manually add the default taint steps. From 8907252814f546607b248793ab65fb4408ae42dc Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 12 Dec 2024 13:53:11 +0100 Subject: [PATCH 14/37] JS: Migrate TemplateObjectInjection --- .../TemplateObjectInjectionCustomizations.qll | 13 ++++++++---- .../dataflow/TemplateObjectInjectionQuery.qll | 20 +++++++++---------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/TemplateObjectInjectionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/TemplateObjectInjectionCustomizations.qll index 5e7ae35dd88f..b141feb1409f 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/TemplateObjectInjectionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/TemplateObjectInjectionCustomizations.qll @@ -12,12 +12,17 @@ private import semmle.javascript.security.TaintedObjectCustomizations * template object injection vulnerabilities. */ module TemplateObjectInjection { + import semmle.javascript.security.CommonFlowState + /** * A data flow source for template object injection vulnerabilities. */ abstract class Source extends DataFlow::Node { - /** Gets a flow label to associate with this source. */ - abstract DataFlow::FlowLabel getAFlowLabel(); + /** Gets a flow state for which this is a source. */ + FlowState getAFlowState() { result.isTaint() } + + /** DEPRECATED. Use `getAFlowState()` instead */ + deprecated DataFlow::FlowLabel getAFlowLabel() { result = this.getAFlowState().toFlowLabel() } } /** @@ -31,12 +36,12 @@ module TemplateObjectInjection { abstract class Sanitizer extends DataFlow::Node { } private class TaintedObjectSourceAsSource extends Source instanceof TaintedObject::Source { - override DataFlow::FlowLabel getAFlowLabel() { result = TaintedObject::label() } + override FlowState getAFlowState() { result.isTaintedObject() } } /** An active threat-model source, considered as a flow source. */ private class ActiveThreatModelSourceAsSource extends Source, ActiveThreatModelSource { - override DataFlow::FlowLabel getAFlowLabel() { result.isTaint() } + override FlowState getAFlowState() { result.isTaint() } } /** diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/TemplateObjectInjectionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/TemplateObjectInjectionQuery.qll index 1a4f02be601f..f71ddfdb6fa1 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/TemplateObjectInjectionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/TemplateObjectInjectionQuery.qll @@ -15,29 +15,29 @@ private import semmle.javascript.security.TaintedObject * A taint tracking configuration for reasoning about template object injection vulnerabilities. */ module TemplateObjectInjectionConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + import semmle.javascript.security.CommonFlowState - predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { - source.(Source).getAFlowLabel() = label + predicate isSource(DataFlow::Node source, FlowState state) { + source.(Source).getAFlowState() = state } - predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - sink instanceof Sink and label = TaintedObject::label() + predicate isSink(DataFlow::Node sink, FlowState state) { + sink instanceof Sink and state.isTaintedObject() } predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } - predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel label) { + predicate isBarrier(DataFlow::Node node, FlowState state) { TaintTracking::defaultSanitizer(node) and - label.isTaint() + state.isTaint() or - node = TaintedObject::SanitizerGuard::getABarrierNode(label) + node = TaintedObject::SanitizerGuard::getABarrierNode(state) } predicate isAdditionalFlowStep( - DataFlow::Node src, DataFlow::FlowLabel inlbl, DataFlow::Node trg, DataFlow::FlowLabel outlbl + DataFlow::Node src, FlowState inlbl, DataFlow::Node trg, FlowState outlbl ) { - TaintedObject::step(src, trg, inlbl, outlbl) + TaintedObject::isAdditionalFlowStep(src, inlbl, trg, outlbl) or // We're not using a taint-tracking config because taint steps would then apply to all flow states. // So we use a plain data flow config and manually add the default taint steps. From d9a43dbd85b769a27ce305559141d19e8944e09e Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 12 Dec 2024 13:57:34 +0100 Subject: [PATCH 15/37] JS: Migrate UnsafeHtmlConstruction --- .../UnsafeHtmlConstructionCustomizations.qll | 11 +++++---- .../dataflow/UnsafeHtmlConstructionQuery.qll | 24 +++++++++---------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionCustomizations.qll index f4ce860c860d..06bad34b80c4 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionCustomizations.qll @@ -13,6 +13,7 @@ module UnsafeHtmlConstruction { private import semmle.javascript.security.dataflow.DomBasedXssCustomizations::DomBasedXss as DomBasedXss private import semmle.javascript.security.dataflow.UnsafeJQueryPluginCustomizations::UnsafeJQueryPlugin as UnsafeJQueryPlugin private import semmle.javascript.PackageExports as Exports + import semmle.javascript.security.CommonFlowState /** * A source for unsafe HTML constructed from library input. @@ -71,16 +72,16 @@ module UnsafeHtmlConstruction { predicate blocksExpr(boolean outcome, Expr e) { none() } /** - * Holds if this node acts as a barrier for `label`, blocking further flow from `e` if `this` evaluates to `outcome`. + * Holds if this node acts as a barrier for `state`, blocking further flow from `e` if `this` evaluates to `outcome`. */ - predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { none() } + predicate blocksExpr(boolean outcome, Expr e, FlowState state) { none() } /** DEPRECATED. Use `blocksExpr` instead. */ deprecated predicate sanitizes(boolean outcome, Expr e) { this.blocksExpr(outcome, e) } /** DEPRECATED. Use `blocksExpr` instead. */ deprecated predicate sanitizes(boolean outcome, Expr e, DataFlow::FlowLabel label) { - this.blocksExpr(outcome, e, label) + this.blocksExpr(outcome, e, FlowState::fromFlowLabel(label)) } } @@ -218,10 +219,10 @@ module UnsafeHtmlConstruction { TypeTestGuard() { TaintTracking::isStringTypeGuard(astNode, operand, polarity) } - override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel lbl) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { polarity = outcome and e = operand and - lbl.isTaint() + state.isTaint() } } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionQuery.qll index dc3b7c96b990..4ca7b4787a43 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionQuery.qll @@ -16,16 +16,16 @@ deprecated class Configration = Configuration; * A taint-tracking configuration for reasoning about unsafe HTML constructed from library input vulnerabilities. */ module UnsafeHtmlConstructionConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + import semmle.javascript.security.CommonFlowState - predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { + predicate isSource(DataFlow::Node source, FlowState state) { source instanceof Source and - label = [TaintedObject::label(), DataFlow::FlowLabel::taint(), DataFlow::FlowLabel::data()] + state = [FlowState::taintedObject(), FlowState::taint()] } - predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { + predicate isSink(DataFlow::Node sink, FlowState state) { sink instanceof Sink and - label = DataFlow::FlowLabel::taint() + state = FlowState::taint() } predicate isBarrier(DataFlow::Node node) { @@ -38,14 +38,14 @@ module UnsafeHtmlConstructionConfig implements DataFlow::StateConfigSig { node = Shared::BarrierGuard::getABarrierNode() } - predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel label) { - TaintTracking::defaultSanitizer(node) and label.isTaint() + predicate isBarrier(DataFlow::Node node, FlowState state) { + TaintTracking::defaultSanitizer(node) and state.isTaint() or - node = DataFlow::MakeLabeledBarrierGuard::getABarrierNode(label) + node = DataFlow::MakeStateBarrierGuard::getABarrierNode(state) } predicate isAdditionalFlowStep( - DataFlow::Node pred, DataFlow::FlowLabel inlbl, DataFlow::Node succ, DataFlow::FlowLabel outlbl + DataFlow::Node pred, FlowState inlbl, DataFlow::Node succ, FlowState outlbl ) { // TODO: localFieldStep is too expensive with dataflow2 // DataFlow::localFieldStep(pred, succ) and @@ -53,12 +53,12 @@ module UnsafeHtmlConstructionConfig implements DataFlow::StateConfigSig { // outlbl.isTaint() none() or - TaintedObject::step(pred, succ, inlbl, outlbl) + TaintedObject::isAdditionalFlowStep(pred, inlbl, succ, outlbl) or // property read from a tainted object is considered tainted succ.(DataFlow::PropRead).getBase() = pred and - inlbl = TaintedObject::label() and - outlbl = DataFlow::FlowLabel::taint() + inlbl.isTaintedObject() and + outlbl.isTaint() or TaintTracking::defaultTaintStep(pred, succ) and inlbl.isTaint() and From 42a7208704d4dcab2bb78cbac31660be00dfd5d3 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 12 Dec 2024 14:13:27 +0100 Subject: [PATCH 16/37] JS: Migrate ExceptionXss --- .../dataflow/ExceptionXssCustomizations.qll | 45 ++++++++++++++++--- .../security/dataflow/ExceptionXssQuery.qll | 22 ++++----- 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssCustomizations.qll index 20a710a9da62..6f865fc54c98 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssCustomizations.qll @@ -14,15 +14,48 @@ import javascript module ExceptionXss { private import Xss::Shared as Shared + private newtype TFlowState = + TThrown() or + TNotYetThrown() + + /** A flow state to associate with a tracked value. */ + class FlowState extends TFlowState { + /** Gets a string representation fo this flow state */ + string toString() { + this = TThrown() and result = "thrown" + or + this = TNotYetThrown() and result = "not-yet-thrown" + } + + deprecated DataFlow::FlowLabel toFlowLabel() { + this = TThrown() and result.isTaint() + or + this = TNotYetThrown() and result instanceof NotYetThrown + } + } + + /** Predicates for working with flow states. */ + module FlowState { + deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } + + /** A tainted value originating from a thrown and caught exception. */ + FlowState thrown() { result = TThrown() } + + /** A value that has not yet been thrown. */ + FlowState notYetThrown() { result = TNotYetThrown() } + } + /** A data flow source for XSS caused by interpreting exception or error text as HTML. */ abstract class Source extends DataFlow::Node { /** - * Gets a flow label to associate with this source. + * Gets a flow state to associate with this source. * * For sources that should pass through a `throw/catch` before reaching the sink, use the - * `NotYetThrown` labe. Otherwise use `taint` (the default). + * `FlowState::notYetThrown()` state. Otherwise use `FlowState::thrown()` (the default). */ - DataFlow::FlowLabel getAFlowLabel() { result.isTaint() } + FlowState getAFlowState() { result = FlowState::thrown() } + + deprecated DataFlow::FlowLabel getAFlowLabel() { result = this.getAFlowState().toFlowLabel() } /** * Gets a human-readable description of what type of error this refers to. @@ -33,17 +66,19 @@ module ExceptionXss { } /** + * DEPRECATED. Use `FlowState` instead. + * * A FlowLabel representing tainted data that has not been thrown in an exception. * In the js/xss-through-exception query data-flow can only reach a sink after * the data has been thrown as an exception, and data that has not been thrown * as an exception therefore has this flow label, and only this flow label, associated with it. */ - abstract class NotYetThrown extends DataFlow::FlowLabel { + abstract deprecated class NotYetThrown extends DataFlow::FlowLabel { NotYetThrown() { this = "NotYetThrown" } } private class XssSourceAsSource extends Source instanceof Shared::Source { - override DataFlow::FlowLabel getAFlowLabel() { result instanceof NotYetThrown } + override FlowState getAFlowState() { result instanceof TNotYetThrown } override string getDescription() { result = "Exception text" } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssQuery.qll index 6908e50960d7..47b65007a5a5 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssQuery.qll @@ -8,6 +8,7 @@ import javascript import DomBasedXssCustomizations::DomBasedXss as DomBasedXssCustom import ReflectedXssCustomizations::ReflectedXss as ReflectedXssCustom import ExceptionXssCustomizations::ExceptionXss +private import ExceptionXssCustomizations::ExceptionXss as ExceptionXss private import semmle.javascript.dataflow.InferredTypes import Xss::Shared as XssShared @@ -71,7 +72,7 @@ predicate canThrowSensitiveInformation(DataFlow::Node node) { } // Materialize flow labels -private class ConcreteNotYetThrown extends NotYetThrown { +deprecated private class ConcreteNotYetThrown extends NotYetThrown { ConcreteNotYetThrown() { this = this } } @@ -130,14 +131,14 @@ private DataFlow::Node getExceptionTarget(DataFlow::Node pred) { * an exception. */ module ExceptionXssConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + class FlowState = ExceptionXss::FlowState; - predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { - source.(Source).getAFlowLabel() = label + predicate isSource(DataFlow::Node source, FlowState state) { + source.(Source).getAFlowState() = state } - predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - sink instanceof XssShared::Sink and not label instanceof NotYetThrown + predicate isSink(DataFlow::Node sink, FlowState state) { + sink instanceof XssShared::Sink and not state = FlowState::notYetThrown() } predicate isBarrier(DataFlow::Node node) { @@ -145,10 +146,10 @@ module ExceptionXssConfig implements DataFlow::StateConfigSig { } predicate isAdditionalFlowStep( - DataFlow::Node pred, DataFlow::FlowLabel inlbl, DataFlow::Node succ, DataFlow::FlowLabel outlbl + DataFlow::Node pred, FlowState inlbl, DataFlow::Node succ, FlowState outlbl ) { - inlbl instanceof NotYetThrown and - (outlbl.isTaint() or outlbl instanceof NotYetThrown) and + inlbl = FlowState::notYetThrown() and + outlbl = [FlowState::thrown(), FlowState::notYetThrown()] and canThrowSensitiveInformation(pred) and succ = getExceptionTarget(pred) } @@ -178,7 +179,8 @@ deprecated class Configuration extends TaintTracking::Configuration { override predicate isAdditionalFlowStep( DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl ) { - ExceptionXssConfig::isAdditionalFlowStep(pred, inlbl, succ, outlbl) + ExceptionXssConfig::isAdditionalFlowStep(pred, FlowState::fromFlowLabel(inlbl), succ, + FlowState::fromFlowLabel(outlbl)) or // All the usual taint-flow steps apply on data-flow before it has been thrown in an exception. // Note: this step is not needed in StateConfigSig module since flow states inherit taint steps. From dc3d7a0159c361119c16d775ac52abedba7a480c Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 10:47:04 +0100 Subject: [PATCH 17/37] Update ExceptionXssCustomizations.qll --- .../javascript/security/dataflow/ExceptionXssCustomizations.qll | 1 + 1 file changed, 1 insertion(+) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssCustomizations.qll index 6f865fc54c98..9c1492be2811 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssCustomizations.qll @@ -55,6 +55,7 @@ module ExceptionXss { */ FlowState getAFlowState() { result = FlowState::thrown() } + /** DEPRECATED. Use `getAFlowState()` instead. */ deprecated DataFlow::FlowLabel getAFlowLabel() { result = this.getAFlowState().toFlowLabel() } /** From 2112ecc44df9b11cfcaa9d66d0bb31d2a330e6a6 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 10:48:43 +0100 Subject: [PATCH 18/37] JS: Migrate HardcodedDataInterpretedAsCode --- ...dedDataInterpretedAsCodeCustomizations.qll | 49 ++++++++++++++++--- .../HardcodedDataInterpretedAsCodeQuery.qll | 16 +++--- 2 files changed, 50 insertions(+), 15 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeCustomizations.qll index 14d2c8fc1489..76549122b868 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeCustomizations.qll @@ -8,20 +8,57 @@ import javascript private import semmle.javascript.security.dataflow.CodeInjectionCustomizations module HardcodedDataInterpretedAsCode { + private newtype TFlowState = + TUnmodified() or + TModified() + + /** A flow state to associate with a tracked value. */ + class FlowState extends TFlowState { + /** Gets a string representation fo this flow state */ + string toString() { + this = TUnmodified() and result = "unmodified" + or + this = TModified() and result = "modified" + } + + deprecated DataFlow::FlowLabel toFlowLabel() { + this = TUnmodified() and result.isData() + or + this = TModified() and result.isTaint() + } + } + + /** Predicates for working with flow states. */ + module FlowState { + deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } + + /** An unmodified value originating from a string constant. */ + FlowState unmodified() { result = TUnmodified() } + + /** A value which has undergone some transformation, such as hex decoding. */ + FlowState modified() { result = TModified() } + } + /** * A data flow source for hard-coded data. */ abstract class Source extends DataFlow::Node { - /** Gets a flow label for which this is a source. */ - DataFlow::FlowLabel getLabel() { result.isData() } + /** Gets a flow state for which this is a source. */ + FlowState getAFlowState() { result = FlowState::unmodified() } + + /** DEPRECATED. Use `getAFlowState()` instead. */ + deprecated DataFlow::FlowLabel getLabel() { result = this.getAFlowState().toFlowLabel() } } /** * A data flow sink for code injection. */ abstract class Sink extends DataFlow::Node { - /** Gets a flow label for which this is a sink. */ - abstract DataFlow::FlowLabel getLabel(); + /** Gets a flow state for which this is a sink. */ + FlowState getAFlowState() { result = FlowState::modified() } + + /** DEPRECATED. Use `getAFlowState()` instead. */ + deprecated DataFlow::FlowLabel getLabel() { result = this.getAFlowState().toFlowLabel() } /** Gets a description of what kind of sink this is. */ abstract string getKind(); @@ -50,7 +87,7 @@ module HardcodedDataInterpretedAsCode { * A code injection sink; hard-coded data should not flow here. */ private class DefaultCodeInjectionSink extends Sink instanceof CodeInjection::Sink { - override DataFlow::FlowLabel getLabel() { result.isTaint() } + override FlowState getAFlowState() { result = FlowState::modified() } override string getKind() { result = "Code" } } @@ -61,7 +98,7 @@ module HardcodedDataInterpretedAsCode { private class RequireArgumentSink extends Sink { RequireArgumentSink() { this = any(Require r).getAnArgument().flow() } - override DataFlow::FlowLabel getLabel() { result.isDataOrTaint() } + override FlowState getAFlowState() { result = [FlowState::modified(), FlowState::unmodified()] } override string getKind() { result = "An import path" } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeQuery.qll index 55ecdbffe804..24c832eb3c32 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeQuery.qll @@ -10,29 +10,27 @@ import javascript import HardcodedDataInterpretedAsCodeCustomizations::HardcodedDataInterpretedAsCode +private import HardcodedDataInterpretedAsCodeCustomizations::HardcodedDataInterpretedAsCode as HardcodedDataInterpretedAsCode /** * A taint-tracking configuration for reasoning about hard-coded data * being interpreted as code */ module HardcodedDataInterpretedAsCodeConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + class FlowState = HardcodedDataInterpretedAsCode::FlowState; - predicate isSource(DataFlow::Node source, DataFlow::FlowLabel lbl) { - source.(Source).getLabel() = lbl - } + predicate isSource(DataFlow::Node source, FlowState lbl) { source.(Source).getAFlowState() = lbl } - predicate isSink(DataFlow::Node nd, DataFlow::FlowLabel lbl) { nd.(Sink).getLabel() = lbl } + predicate isSink(DataFlow::Node nd, FlowState lbl) { nd.(Sink).getAFlowState() = lbl } predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } predicate isAdditionalFlowStep( - DataFlow::Node node1, DataFlow::FlowLabel state1, DataFlow::Node node2, - DataFlow::FlowLabel state2 + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { TaintTracking::defaultTaintStep(node1, node2) and - state1.isDataOrTaint() and - state2.isTaint() + state1 = [FlowState::modified(), FlowState::unmodified()] and + state2 = FlowState::modified() } } From d381ab1260a5258ca97ce9a231234af4b1c87926 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 10:55:00 +0100 Subject: [PATCH 19/37] JS: Migrate IncompleteHtmlAttributeSanitization --- ...eHtmlAttributeSanitizationCustomizations.qll | 14 ++++++++++++++ ...IncompleteHtmlAttributeSanitizationQuery.qll | 17 +++++++++-------- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/IncompleteHtmlAttributeSanitizationCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/IncompleteHtmlAttributeSanitizationCustomizations.qll index 37bc2ae0bb79..f421a92298f9 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/IncompleteHtmlAttributeSanitizationCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/IncompleteHtmlAttributeSanitizationCustomizations.qll @@ -8,6 +8,20 @@ import javascript import semmle.javascript.security.IncompleteBlacklistSanitizer module IncompleteHtmlAttributeSanitization { + private newtype TFlowState = TCharacter(string c) { c = ["\"", "'", "&"] } + + /** A flow state to associate with a tracked value. */ + class FlowState extends TFlowState { + /** Gets a string representation of this flow state. */ + string toString() { this = TCharacter(result) } + } + + /** Predicates for working with flow states. */ + module FlowState { + /** Gets the flow state corresponding to `c`. */ + FlowState character(string c) { result = TCharacter(c) } + } + /** * A data flow source for incomplete HTML sanitization vulnerabilities. */ diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/IncompleteHtmlAttributeSanitizationQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/IncompleteHtmlAttributeSanitizationQuery.qll index 824d689445ea..6731754206b1 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/IncompleteHtmlAttributeSanitizationQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/IncompleteHtmlAttributeSanitizationQuery.qll @@ -9,8 +9,9 @@ import javascript import IncompleteHtmlAttributeSanitizationCustomizations::IncompleteHtmlAttributeSanitization +private import IncompleteHtmlAttributeSanitizationCustomizations::IncompleteHtmlAttributeSanitization as IncompleteHtmlAttributeSanitization -private module Label { +deprecated private module Label { class Quote extends DataFlow::FlowLabel { Quote() { this = ["\"", "'"] } } @@ -26,18 +27,18 @@ private module Label { * A taint-tracking configuration for reasoning about incomplete HTML sanitization vulnerabilities. */ module IncompleteHtmlAttributeSanitizationConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + class FlowState = IncompleteHtmlAttributeSanitization::FlowState; - predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { - label = Label::characterToLabel(source.(Source).getAnUnsanitizedCharacter()) + predicate isSource(DataFlow::Node source, FlowState label) { + label = FlowState::character(source.(Source).getAnUnsanitizedCharacter()) } - predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - label = Label::characterToLabel(sink.(Sink).getADangerousCharacter()) + predicate isSink(DataFlow::Node sink, FlowState label) { + label = FlowState::character(sink.(Sink).getADangerousCharacter()) } - predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel lbl) { - lbl = Label::characterToLabel(node.(StringReplaceCall).getAReplacedString()) + predicate isBarrier(DataFlow::Node node, FlowState lbl) { + lbl = FlowState::character(node.(StringReplaceCall).getAReplacedString()) } predicate isBarrier(DataFlow::Node n) { n instanceof Sanitizer } From 4e25036cdcb23a43efa5153888ffa7210d2b7590 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 11:09:59 +0100 Subject: [PATCH 20/37] JS: Follow naming convention in InsecureModuleFlow module --- .../javascript/security/dataflow/InsecureDownloadQuery.qll | 2 +- javascript/ql/src/Security/CWE-829/InsecureDownload.ql | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadQuery.qll index 7f7d3341d5ae..77a08db5d730 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadQuery.qll @@ -29,7 +29,7 @@ module InsecureDownloadConfig implements DataFlow::StateConfigSig { /** * Taint tracking for download of sensitive file through insecure connection. */ -module InsecureDownload = DataFlow::GlobalWithState; +module InsecureDownloadFlow = DataFlow::GlobalWithState; /** * DEPRECATED. Use the `InsecureDownload` module instead. diff --git a/javascript/ql/src/Security/CWE-829/InsecureDownload.ql b/javascript/ql/src/Security/CWE-829/InsecureDownload.ql index 4644f9813927..b040645eacd2 100644 --- a/javascript/ql/src/Security/CWE-829/InsecureDownload.ql +++ b/javascript/ql/src/Security/CWE-829/InsecureDownload.ql @@ -13,9 +13,9 @@ import javascript import semmle.javascript.security.dataflow.InsecureDownloadQuery -import DataFlow::DeduplicatePathGraph +import DataFlow::DeduplicatePathGraph from PathNode source, PathNode sink -where InsecureDownload::flowPath(source.getAnOriginalPathNode(), sink.getAnOriginalPathNode()) +where InsecureDownloadFlow::flowPath(source.getAnOriginalPathNode(), sink.getAnOriginalPathNode()) select sink.getNode(), source, sink, "$@ of sensitive file from $@.", sink.getNode().(Sink).getDownloadCall(), "Download", source.getNode(), "HTTP source" From bcc1669f4c1a1cd5bf7751de971313ce600ca1c5 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 11:10:14 +0100 Subject: [PATCH 21/37] JS: Migrate InsecureDownload --- .../InsecureDownloadCustomizations.qll | 65 +++++++++++++++---- .../dataflow/InsecureDownloadQuery.qll | 15 ++--- 2 files changed, 61 insertions(+), 19 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadCustomizations.qll index dc383df448c3..d8e66e65122b 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadCustomizations.qll @@ -10,14 +10,52 @@ import javascript * Classes and predicates for reasoning about download of sensitive file through insecure connection vulnerabilities. */ module InsecureDownload { + private newtype TFlowState = + TSensitiveInsecureUrl() or + TInsecureUrl() + + /** A flow state to associate with a tracked value. */ + class FlowState extends TFlowState { + /** Gets a string representation fo this flow state */ + string toString() { + this = TSensitiveInsecureUrl() and result = "sensitive-insecure-url" + or + this = TInsecureUrl() and result = "insecure-url" + } + + deprecated DataFlow::FlowLabel toFlowLabel() { + this = TSensitiveInsecureUrl() and result instanceof Label::SensitiveInsecureUrl + or + this = TInsecureUrl() and result instanceof Label::InsecureUrl + } + } + + /** Predicates for working with flow states. */ + module FlowState { + deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } + + /** + * A file URL that is both sensitive and downloaded over an insecure connection. + */ + FlowState sensitiveInsecureUrl() { result = TSensitiveInsecureUrl() } + + /** + * A URL that is downloaded over an insecure connection. + */ + FlowState insecureUrl() { result = TInsecureUrl() } + } + /** * A data flow source for download of sensitive file through insecure connection. */ abstract class Source extends DataFlow::Node { /** - * Gets a flow-label for this source. + * Gets a flow state for this source. */ - abstract DataFlow::FlowLabel getALabel(); + FlowState getAFlowState() { result = FlowState::insecureUrl() } + + /** DEPRECATED. Use `getAFlowState()` instead. */ + deprecated DataFlow::FlowLabel getALabel() { result = this.getAFlowState().toFlowLabel() } } /** @@ -30,9 +68,14 @@ module InsecureDownload { abstract DataFlow::Node getDownloadCall(); /** - * Gets a flow-label where this sink is vulnerable. + * Gets a flow state where this sink is vulnerable. */ - abstract DataFlow::FlowLabel getALabel(); + FlowState getAFlowState() { + result = [FlowState::insecureUrl(), FlowState::sensitiveInsecureUrl()] + } + + /** DEPRECATED. Use `getAFlowState()` instead. */ + deprecated DataFlow::FlowLabel getALabel() { result = this.getAFlowState().toFlowLabel() } } /** @@ -71,11 +114,11 @@ module InsecureDownload { str.regexpMatch("http://.*|ftp://.*") } - override DataFlow::FlowLabel getALabel() { - result instanceof Label::InsecureUrl + override FlowState getAFlowState() { + result = FlowState::insecureUrl() or hasUnsafeExtension(str) and - result instanceof Label::SensitiveInsecureUrl + result = FlowState::sensitiveInsecureUrl() } } @@ -113,11 +156,11 @@ module InsecureDownload { override DataFlow::Node getDownloadCall() { result = request } - override DataFlow::FlowLabel getALabel() { - result instanceof Label::SensitiveInsecureUrl + override FlowState getAFlowState() { + result = FlowState::sensitiveInsecureUrl() or hasUnsafeExtension(request.getASavePath().getStringValue()) and - result instanceof Label::InsecureUrl + result = FlowState::insecureUrl() } } @@ -145,7 +188,7 @@ module InsecureDownload { ) } - override DataFlow::FlowLabel getALabel() { result instanceof Label::InsecureUrl } + override FlowState getAFlowState() { result = FlowState::insecureUrl() } override DataFlow::Node getDownloadCall() { result = request } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadQuery.qll index 77a08db5d730..6a633ec324e5 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadQuery.qll @@ -8,20 +8,19 @@ import javascript import InsecureDownloadCustomizations::InsecureDownload +private import InsecureDownloadCustomizations::InsecureDownload as InsecureDownload /** * A taint tracking configuration for download of sensitive file through insecure connection. */ module InsecureDownloadConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + class FlowState = InsecureDownload::FlowState; - predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { - source.(Source).getALabel() = label + predicate isSource(DataFlow::Node source, FlowState state) { + source.(Source).getAFlowState() = state } - predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - sink.(Sink).getALabel() = label - } + predicate isSink(DataFlow::Node sink, FlowState state) { sink.(Sink).getAFlowState() = state } predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } } @@ -38,11 +37,11 @@ deprecated class Configuration extends DataFlow::Configuration { Configuration() { this = "InsecureDownload" } override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { - InsecureDownloadConfig::isSource(source, label) + InsecureDownloadConfig::isSource(source, FlowState::fromFlowLabel(label)) } override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - InsecureDownloadConfig::isSink(sink, label) + InsecureDownloadConfig::isSink(sink, FlowState::fromFlowLabel(label)) } override predicate isBarrier(DataFlow::Node node) { From a9e89ed8e3b95111ebb1db1cd52d456ce9b0b167 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 11:23:31 +0100 Subject: [PATCH 22/37] JS: Migrate PrototypePollutingAssignment --- ...otypePollutingAssignmentCustomizations.qll | 46 ++++++++++++-- .../PrototypePollutingAssignmentQuery.qll | 62 +++++++++---------- 2 files changed, 71 insertions(+), 37 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentCustomizations.qll index 5ac278924e24..a196cac42178 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentCustomizations.qll @@ -10,6 +10,37 @@ private import javascript * that my cause prototype pollution. */ module PrototypePollutingAssignment { + private newtype TFlowState = + TTaint() or + TObjectPrototype() + + /** A flow state to associate with a tracked value. */ + class FlowState extends TFlowState { + /** Gets a string representation fo this flow state */ + string toString() { + this = TTaint() and result = "taint" + or + this = TObjectPrototype() and result = "object-prototype" + } + + deprecated DataFlow::FlowLabel toFlowLabel() { + this = TTaint() and result.isTaint() + or + this = TObjectPrototype() and result instanceof ObjectPrototype + } + } + + /** Predicates for working with flow states. */ + module FlowState { + deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } + + /** A tainted value. */ + FlowState taint() { result = TTaint() } + + /** A reference to `Object.prototype` obtained by reading from a tainted property name. */ + FlowState objectPrototype() { result = TObjectPrototype() } + } + /** * A data flow source for untrusted data from which the special `__proto__` property name may be arise. */ @@ -30,7 +61,10 @@ module PrototypePollutingAssignment { * Use the `taint` label for untrusted property names, and the `ObjectPrototype` label for * object mutations. */ - abstract DataFlow::FlowLabel getAFlowLabel(); + FlowState getAFlowState() { result = FlowState::objectPrototype() } + + /** DEPRECATED. Use `getAFlowState()` instead. */ + deprecated DataFlow::FlowLabel getAFlowLabel() { result = this.getAFlowState().toFlowLabel() } } /** @@ -48,16 +82,16 @@ module PrototypePollutingAssignment { predicate blocksExpr(boolean outcome, Expr e) { none() } /** - * Holds if this node acts as a barrier for `label`, blocking further flow from `e` if `this` evaluates to `outcome`. + * Holds if this node acts as a barrier for `state`, blocking further flow from `e` if `this` evaluates to `outcome`. */ - predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { none() } + predicate blocksExpr(boolean outcome, Expr e, FlowState state) { none() } /** DEPRECATED. Use `blocksExpr` instead. */ deprecated predicate sanitizes(boolean outcome, Expr e) { this.blocksExpr(outcome, e) } /** DEPRECATED. Use `blocksExpr` instead. */ deprecated predicate sanitizes(boolean outcome, Expr e, DataFlow::FlowLabel label) { - this.blocksExpr(outcome, e, label) + this.blocksExpr(outcome, e, FlowState::fromFlowLabel(label)) } } @@ -74,7 +108,7 @@ module PrototypePollutingAssignment { } /** A flow label representing the `Object.prototype` value. */ - abstract class ObjectPrototype extends DataFlow::FlowLabel { + abstract deprecated class ObjectPrototype extends DataFlow::FlowLabel { ObjectPrototype() { this = "Object.prototype" } } @@ -90,7 +124,7 @@ module PrototypePollutingAssignment { this = any(DeleteExpr del).getOperand().flow().(DataFlow::PropRef).getBase() } - override DataFlow::FlowLabel getAFlowLabel() { result instanceof ObjectPrototype } + override FlowState getAFlowState() { result = FlowState::objectPrototype() } } /** A remote flow source or location.{hash,search} as a taint source. */ diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentQuery.qll index 517da8f7c023..34a7c3a83462 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentQuery.qll @@ -11,24 +11,23 @@ private import javascript private import semmle.javascript.DynamicPropertyAccess private import semmle.javascript.dataflow.InferredTypes import PrototypePollutingAssignmentCustomizations::PrototypePollutingAssignment +private import PrototypePollutingAssignmentCustomizations::PrototypePollutingAssignment as PrototypePollutingAssignment private import semmle.javascript.filters.ClassifyFiles as ClassifyFiles // Materialize flow labels -private class ConcreteObjectPrototype extends ObjectPrototype { +deprecated private class ConcreteObjectPrototype extends ObjectPrototype { ConcreteObjectPrototype() { this = this } } /** A taint-tracking configuration for reasoning about prototype-polluting assignments. */ module PrototypePollutingAssignmentConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + class FlowState = PrototypePollutingAssignment::FlowState; - predicate isSource(DataFlow::Node node, DataFlow::FlowLabel label) { - node instanceof Source and label.isTaint() + predicate isSource(DataFlow::Node node, FlowState label) { + node instanceof Source and label = FlowState::taint() } - predicate isSink(DataFlow::Node node, DataFlow::FlowLabel lbl) { - node.(Sink).getAFlowLabel() = lbl - } + predicate isSink(DataFlow::Node node, FlowState lbl) { node.(Sink).getAFlowState() = lbl } predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer @@ -59,28 +58,28 @@ module PrototypePollutingAssignmentConfig implements DataFlow::StateConfigSig { node = DataFlow::MakeBarrierGuard::getABarrierNode() } - predicate isBarrierOut(DataFlow::Node node, DataFlow::FlowLabel lbl) { + predicate isBarrierOut(DataFlow::Node node, FlowState lbl) { // Suppress the value-preserving step src -> dst in `extend(dst, src)`. This is modeled as a value-preserving // step because it preserves all properties, but the destination is not actually Object.prototype. node = any(ExtendCall call).getASourceOperand() and - lbl instanceof ObjectPrototype + lbl = FlowState::objectPrototype() } - predicate isBarrierIn(DataFlow::Node node, DataFlow::FlowLabel lbl) { + predicate isBarrierIn(DataFlow::Node node, FlowState lbl) { // FIXME: This should only be an in-barrier for the corresponding flow state, but flow-state specific in-barriers are not supported right now. isSource(node, lbl) } predicate isAdditionalFlowStep( - DataFlow::Node pred, DataFlow::FlowLabel inlbl, DataFlow::Node succ, DataFlow::FlowLabel outlbl + DataFlow::Node pred, FlowState inlbl, DataFlow::Node succ, FlowState outlbl ) { // Step from x -> obj[x] while switching to the ObjectPrototype label // (If `x` can have the value `__proto__` then the result can be Object.prototype) exists(DynamicPropRead read | pred = read.getPropertyNameNode() and succ = read and - inlbl.isTaint() and - outlbl instanceof ObjectPrototype and + inlbl = FlowState::taint() and + outlbl = FlowState::objectPrototype() and // Exclude cases where the property name came from a property enumeration. // If the property name is an own property of the base object, the read won't // return Object.prototype. @@ -96,8 +95,8 @@ module PrototypePollutingAssignmentConfig implements DataFlow::StateConfigSig { proj.isSingletonProjection() and pred = proj.getASelector() and succ = proj and - inlbl.isTaint() and - outlbl instanceof ObjectPrototype + inlbl = FlowState::taint() and + outlbl = FlowState::objectPrototype() ) or // TODO: local field step becomes a jump step, resulting in FPs (closure-lib) @@ -105,22 +104,22 @@ module PrototypePollutingAssignmentConfig implements DataFlow::StateConfigSig { // DataFlow::localFieldStep(pred, succ) none() or - inlbl.isTaint() and + inlbl = FlowState::taint() and TaintTracking::defaultTaintStep(pred, succ) and inlbl = outlbl } DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext } - predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel lbl) { - lbl.isTaint() and + predicate isBarrier(DataFlow::Node node, FlowState lbl) { + lbl = FlowState::taint() and TaintTracking::defaultSanitizer(node) or // Don't propagate into the receiver, as the method lookups will generally fail on Object.prototype. node instanceof DataFlow::ThisNode and - lbl instanceof ObjectPrototype + lbl = FlowState::objectPrototype() or - node = DataFlow::MakeLabeledBarrierGuard::getABarrierNode(lbl) + node = DataFlow::MakeStateBarrierGuard::getABarrierNode(lbl) } } @@ -173,7 +172,8 @@ deprecated class Configuration extends TaintTracking::Configuration { override predicate isAdditionalFlowStep( DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl ) { - PrototypePollutingAssignmentConfig::isAdditionalFlowStep(pred, inlbl, succ, outlbl) + PrototypePollutingAssignmentConfig::isAdditionalFlowStep(pred, FlowState::fromFlowLabel(inlbl), + succ, FlowState::fromFlowLabel(outlbl)) } override predicate hasFlowPath(DataFlow::SourcePathNode source, DataFlow::SinkPathNode sink) { @@ -264,10 +264,10 @@ private class PropertyPresenceCheck extends BarrierGuard, DataFlow::ValueNode { not isPropertyPresentOnObjectPrototype(astNode.getPropertyName()) } - override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState label) { e = astNode.getBase() and outcome = true and - label instanceof ObjectPrototype + label = FlowState::objectPrototype() } } @@ -279,10 +279,10 @@ private class InExprCheck extends BarrierGuard, DataFlow::ValueNode { not isPropertyPresentOnObjectPrototype(astNode.getLeftOperand().getStringValue()) } - override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState label) { e = astNode.getRightOperand() and outcome = true and - label instanceof ObjectPrototype + label = FlowState::objectPrototype() } } @@ -290,10 +290,10 @@ private class InExprCheck extends BarrierGuard, DataFlow::ValueNode { private class InstanceofCheck extends BarrierGuard, DataFlow::ValueNode { override InstanceofExpr astNode; - override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState label) { e = astNode.getLeftOperand() and outcome = true and - label instanceof ObjectPrototype + label = FlowState::objectPrototype() } } @@ -311,10 +311,10 @@ private class TypeofCheck extends BarrierGuard, DataFlow::ValueNode { ) } - override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState label) { polarity = outcome and e = operand and - label instanceof ObjectPrototype + label = FlowState::objectPrototype() } } @@ -332,10 +332,10 @@ class NumberGuard extends BarrierGuard instanceof DataFlow::CallNode { private class IsArrayCheck extends BarrierGuard, DataFlow::CallNode { IsArrayCheck() { this = DataFlow::globalVarRef("Array").getAMemberCall("isArray") } - override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState label) { e = this.getArgument(0).asExpr() and outcome = true and - label instanceof ObjectPrototype + label = FlowState::objectPrototype() } } From 820f81fc10194e1b978890afd6388d1c9460eadf Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 11:32:25 +0100 Subject: [PATCH 23/37] JS: Migrate UnsafeDynamicMethodAccess --- ...nsafeDynamicMethodAccessCustomizations.qll | 55 ++++++++++++++++--- .../UnsafeDynamicMethodAccessQuery.qll | 40 +++++++------- 2 files changed, 66 insertions(+), 29 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessCustomizations.qll index 3c5cc713e6eb..711157a8d3e7 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessCustomizations.qll @@ -10,16 +10,48 @@ import semmle.javascript.frameworks.Express import PropertyInjectionShared module UnsafeDynamicMethodAccess { - private import DataFlow::FlowLabel + private newtype TFlowState = + TTaint() or + TUnsafeFunction() + + /** A flow state to associate with a tracked value. */ + class FlowState extends TFlowState { + /** Gets a string representation fo this flow state */ + string toString() { + this = TTaint() and result = "taint" + or + this = TUnsafeFunction() and result = "unsafe-function" + } + + deprecated DataFlow::FlowLabel toFlowLabel() { + this = TTaint() and result.isTaint() + or + this = TUnsafeFunction() and result instanceof UnsafeFunction + } + } + + /** Predicates for working with flow states. */ + module FlowState { + deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } + + /** A tainted value. */ + FlowState taint() { result = TTaint() } + + /** A reference to an unsafe function, such as `eval`, obtained by reading from a tainted property name. */ + FlowState unsafeFunction() { result = TUnsafeFunction() } + } /** * A data flow source for unsafe dynamic method access. */ abstract class Source extends DataFlow::Node { /** - * Gets the flow label relevant for this source. + * Gets a flow state relevant for this source. */ - DataFlow::FlowLabel getFlowLabel() { result = taint() } + FlowState getAFlowState() { result = FlowState::taint() } + + /** DEPRECATED. Use `getAFlowState()` instead. */ + deprecated DataFlow::FlowLabel getFlowLabel() { result = this.getAFlowState().toFlowLabel() } } /** @@ -27,9 +59,12 @@ module UnsafeDynamicMethodAccess { */ abstract class Sink extends DataFlow::Node { /** - * Gets the flow label relevant for this sink + * Gets a flow state relevant for this sink. */ - abstract DataFlow::FlowLabel getFlowLabel(); + FlowState getAFlowState() { result = FlowState::taint() } + + /** DEPRECATED. Use `getAFlowState()` instead. */ + deprecated DataFlow::FlowLabel getFlowLabel() { result = this.getAFlowState().toFlowLabel() } } /** @@ -38,16 +73,20 @@ module UnsafeDynamicMethodAccess { abstract class Sanitizer extends DataFlow::Node { } /** + * DEPRECATED. Use `FlowState::unsafeFunction()` instead. + * * Gets the flow label describing values that may refer to an unsafe * function as a result of an attacker-controlled property name. */ - UnsafeFunction unsafeFunction() { any() } + deprecated UnsafeFunction unsafeFunction() { any() } /** + * DEPRECATED. Use `FlowState::unsafeFunction()` instead. + * * A flow label describing values that may refer to an unsafe * function as a result of an attacker-controlled property name. */ - abstract class UnsafeFunction extends DataFlow::FlowLabel { + abstract deprecated class UnsafeFunction extends DataFlow::FlowLabel { UnsafeFunction() { this = "UnsafeFunction" } } @@ -67,6 +106,6 @@ module UnsafeDynamicMethodAccess { class CalleeAsSink extends Sink { CalleeAsSink() { this = any(DataFlow::InvokeNode node).getCalleeNode() } - override DataFlow::FlowLabel getFlowLabel() { result = unsafeFunction() } + override FlowState getAFlowState() { result = FlowState::unsafeFunction() } } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessQuery.qll index f73363d1767d..57ea77d1d4a3 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessQuery.qll @@ -11,9 +11,10 @@ import javascript import PropertyInjectionShared private import DataFlow::FlowLabel import UnsafeDynamicMethodAccessCustomizations::UnsafeDynamicMethodAccess +private import UnsafeDynamicMethodAccessCustomizations::UnsafeDynamicMethodAccess as UnsafeDynamicMethodAccess // Materialize flow labels -private class ConcreteUnsafeFunction extends UnsafeFunction { +deprecated private class ConcreteUnsafeFunction extends UnsafeFunction { ConcreteUnsafeFunction() { this = this } } @@ -21,15 +22,13 @@ private class ConcreteUnsafeFunction extends UnsafeFunction { * A taint-tracking configuration for reasoning about unsafe dynamic method access. */ module UnsafeDynamicMethodAccessConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + class FlowState = UnsafeDynamicMethodAccess::FlowState; - predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { - source.(Source).getFlowLabel() = label + predicate isSource(DataFlow::Node source, FlowState label) { + source.(Source).getAFlowState() = label } - predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - sink.(Sink).getFlowLabel() = label - } + predicate isSink(DataFlow::Node sink, FlowState label) { sink.(Sink).getAFlowState() = label } predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer @@ -38,23 +37,22 @@ module UnsafeDynamicMethodAccessConfig implements DataFlow::StateConfigSig { not StringConcatenation::isCoercion(node) } - predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel label) { + predicate isBarrier(DataFlow::Node node, FlowState label) { TaintTracking::defaultSanitizer(node) and - label.isTaint() + label = FlowState::taint() } /** An additional flow step for use in both this configuration and the legacy configuration. */ additional predicate additionalFlowStep( - DataFlow::Node src, DataFlow::FlowLabel srclabel, DataFlow::Node dst, - DataFlow::FlowLabel dstlabel + DataFlow::Node src, FlowState srclabel, DataFlow::Node dst, FlowState dstlabel ) { // Reading a property of the global object or of a function exists(DataFlow::PropRead read | PropertyInjection::hasUnsafeMethods(read.getBase().getALocalSource()) and src = read.getPropertyNameExpr().flow() and dst = read and - srclabel.isTaint() and - dstlabel = unsafeFunction() + srclabel = FlowState::taint() and + dstlabel = FlowState::unsafeFunction() ) or // Reading a chain of properties from any object with a prototype can lead to Function @@ -62,20 +60,19 @@ module UnsafeDynamicMethodAccessConfig implements DataFlow::StateConfigSig { not PropertyInjection::isPrototypeLessObject(proj.getObject().getALocalSource()) and src = proj.getASelector() and dst = proj and - srclabel.isTaint() and - dstlabel = unsafeFunction() + srclabel = FlowState::taint() and + dstlabel = FlowState::unsafeFunction() ) } predicate isAdditionalFlowStep( - DataFlow::Node src, DataFlow::FlowLabel srclabel, DataFlow::Node dst, - DataFlow::FlowLabel dstlabel + DataFlow::Node src, FlowState srclabel, DataFlow::Node dst, FlowState dstlabel ) { additionalFlowStep(src, srclabel, dst, dstlabel) or // We're not using a taint-tracking config because taint steps would then apply to all flow states. // So we use a plain data flow config and manually add the default taint steps. - srclabel.isTaint() and + srclabel = FlowState::taint() and TaintTracking::defaultTaintStep(src, dst) and srclabel = dstlabel } @@ -93,11 +90,11 @@ deprecated class Configuration extends TaintTracking::Configuration { Configuration() { this = "UnsafeDynamicMethodAccess" } override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { - UnsafeDynamicMethodAccessConfig::isSource(source, label) + UnsafeDynamicMethodAccessConfig::isSource(source, FlowState::fromFlowLabel(label)) } override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - UnsafeDynamicMethodAccessConfig::isSink(sink, label) + UnsafeDynamicMethodAccessConfig::isSink(sink, FlowState::fromFlowLabel(label)) } override predicate isSanitizer(DataFlow::Node node) { @@ -117,6 +114,7 @@ deprecated class Configuration extends TaintTracking::Configuration { DataFlow::Node src, DataFlow::Node dst, DataFlow::FlowLabel srclabel, DataFlow::FlowLabel dstlabel ) { - UnsafeDynamicMethodAccessConfig::additionalFlowStep(src, srclabel, dst, dstlabel) + UnsafeDynamicMethodAccessConfig::additionalFlowStep(src, FlowState::fromFlowLabel(srclabel), + dst, FlowState::fromFlowLabel(dstlabel)) } } From c951a29e2a8a255117f79bec14b180d996f70108 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 11:47:25 +0100 Subject: [PATCH 24/37] JS: Migrate UnvalidatedDynamicMethodCall --- ...lidatedDynamicMethodCallCustomizations.qll | 84 +++++++++++++++---- .../UnvalidatedDynamicMethodCallQuery.qll | 42 +++++----- 2 files changed, 87 insertions(+), 39 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll index 00542294d887..19240f94a110 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll @@ -10,16 +10,60 @@ import PropertyInjectionShared private import semmle.javascript.dataflow.InferredTypes module UnvalidatedDynamicMethodCall { - private import DataFlow::FlowLabel + private newtype TFlowState = + TTaint() or + TMaybeNonFunction() or + TMaybeFromProto() + + /** A flow state to associate with a tracked value. */ + class FlowState extends TFlowState { + /** Gets a string representation fo this flow state */ + string toString() { + this = TTaint() and result = "taint" + or + this = TMaybeNonFunction() and result = "maybe-non-function" + or + this = TMaybeFromProto() and result = "maybe-from-proto" + } + + deprecated DataFlow::FlowLabel toFlowLabel() { + this = TTaint() and result.isTaint() + or + this = TMaybeNonFunction() and result instanceof MaybeNonFunction + or + this = TMaybeFromProto() and result instanceof MaybeFromProto + } + } + + /** Predicates for working with flow states. */ + module FlowState { + deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } + + /** A tainted value. */ + FlowState taint() { result = TTaint() } + + /** + * A non-function value, obtained by reading from a tainted property name. + */ + FlowState maybeNonFunction() { result = TMaybeNonFunction() } + + /** + * A value obtained from a prototype object while reading from a tainted property name. + */ + FlowState maybeFromProto() { result = TMaybeFromProto() } + } /** * A data flow source for unvalidated dynamic method calls. */ abstract class Source extends DataFlow::Node { /** - * Gets the flow label relevant for this source. + * Gets the flow state relevant for this source. */ - DataFlow::FlowLabel getFlowLabel() { result = taint() } + FlowState getAFlowState() { result = FlowState::taint() } + + /** DEPRECATED. Use `getAFlowState()` instead. */ + deprecated DataFlow::FlowLabel getFlowLabel() { result = this.getAFlowState().toFlowLabel() } } /** @@ -27,9 +71,12 @@ module UnvalidatedDynamicMethodCall { */ abstract class Sink extends DataFlow::Node { /** - * Gets the flow label relevant for this sink + * Gets the flow state relevant for this sink */ - abstract DataFlow::FlowLabel getFlowLabel(); + FlowState getAFlowState() { result = FlowState::taint() } + + /** DEPRECATED. Use `getAFlowState()` instead. */ + deprecated DataFlow::FlowLabel getFlowLabel() { result = this.getAFlowState().toFlowLabel() } } /** @@ -37,9 +84,12 @@ module UnvalidatedDynamicMethodCall { */ abstract class Sanitizer extends DataFlow::Node { /** - * Gets the flow label blocked by this sanitizer. + * Gets a flow state blocked by this sanitizer. */ - DataFlow::FlowLabel getFlowLabel() { result.isTaint() } + FlowState getAFlowState() { result = FlowState::taint() } + + /** DEPRECATED. Use `getAFlowState()` instead. */ + deprecated DataFlow::FlowLabel getFlowLabel() { result = this.getAFlowState().toFlowLabel() } /** * DEPRECATED. Use sanitizer nodes instead. @@ -64,16 +114,16 @@ module UnvalidatedDynamicMethodCall { predicate blocksExpr(boolean outcome, Expr e) { none() } /** - * Holds if this node acts as a barrier for `label`, blocking further flow from `e` if `this` evaluates to `outcome`. + * Holds if this node acts as a barrier for `state`, blocking further flow from `e` if `this` evaluates to `outcome`. */ - predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { none() } + predicate blocksExpr(boolean outcome, Expr e, FlowState state) { none() } /** DEPRECATED. Use `blocksExpr` instead. */ deprecated predicate sanitizes(boolean outcome, Expr e) { this.blocksExpr(outcome, e) } /** DEPRECATED. Use `blocksExpr` instead. */ deprecated predicate sanitizes(boolean outcome, Expr e, DataFlow::FlowLabel label) { - this.blocksExpr(outcome, e, label) + this.blocksExpr(outcome, e, FlowState::fromFlowLabel(label)) } } @@ -93,7 +143,7 @@ module UnvalidatedDynamicMethodCall { * A flow label describing values read from a user-controlled property that * may not be functions. */ - abstract class MaybeNonFunction extends DataFlow::FlowLabel { + abstract deprecated class MaybeNonFunction extends DataFlow::FlowLabel { MaybeNonFunction() { this = "MaybeNonFunction" } } @@ -101,7 +151,7 @@ module UnvalidatedDynamicMethodCall { * A flow label describing values read from a user-controlled property that * may originate from a prototype object. */ - abstract class MaybeFromProto extends DataFlow::FlowLabel { + abstract deprecated class MaybeFromProto extends DataFlow::FlowLabel { MaybeFromProto() { this = "MaybeFromProto" } } @@ -134,14 +184,14 @@ module UnvalidatedDynamicMethodCall { ) } - override DataFlow::FlowLabel getFlowLabel() { - result instanceof MaybeNonFunction and + override FlowState getAFlowState() { + result = FlowState::maybeNonFunction() and // don't flag if the type inference can prove that it is a function; // this complements the `FunctionCheck` sanitizer below: the type inference can // detect more checks locally, but doesn't provide inter-procedural reasoning this.analyze().getAType() != TTFunction() or - result instanceof MaybeFromProto + result = FlowState::maybeFromProto() } } @@ -155,10 +205,10 @@ module UnvalidatedDynamicMethodCall { FunctionCheck() { TaintTracking::isTypeofGuard(astNode, operand, "function") } - override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { outcome = astNode.getPolarity() and e = operand and - label instanceof MaybeNonFunction + state = FlowState::maybeNonFunction() } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallQuery.qll index e964770437d0..fdc8155deb34 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallQuery.qll @@ -13,14 +13,14 @@ import semmle.javascript.frameworks.Express import PropertyInjectionShared private import semmle.javascript.dataflow.InferredTypes import UnvalidatedDynamicMethodCallCustomizations::UnvalidatedDynamicMethodCall -private import DataFlow::FlowLabel +private import UnvalidatedDynamicMethodCallCustomizations::UnvalidatedDynamicMethodCall as UnvalidatedDynamicMethodCall // Materialize flow labels -private class ConcreteMaybeNonFunction extends MaybeNonFunction { +deprecated private class ConcreteMaybeNonFunction extends MaybeNonFunction { ConcreteMaybeNonFunction() { this = this } } -private class ConcreteMaybeFromProto extends MaybeFromProto { +deprecated private class ConcreteMaybeFromProto extends MaybeFromProto { ConcreteMaybeFromProto() { this = this } } @@ -28,23 +28,21 @@ private class ConcreteMaybeFromProto extends MaybeFromProto { * A taint-tracking configuration for reasoning about unvalidated dynamic method calls. */ module UnvalidatedDynamicMethodCallConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + class FlowState = UnvalidatedDynamicMethodCall::FlowState; - predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { - source.(Source).getFlowLabel() = label + predicate isSource(DataFlow::Node source, FlowState label) { + source.(Source).getAFlowState() = label } - predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - sink.(Sink).getFlowLabel() = label - } + predicate isSink(DataFlow::Node sink, FlowState label) { sink.(Sink).getAFlowState() = label } - predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel label) { - node.(Sanitizer).getFlowLabel() = label + predicate isBarrier(DataFlow::Node node, FlowState label) { + node.(Sanitizer).getAFlowState() = label or TaintTracking::defaultSanitizer(node) and - label.isTaint() + label = FlowState::taint() or - node = DataFlow::MakeLabeledBarrierGuard::getABarrierNode(label) + node = DataFlow::MakeStateBarrierGuard::getABarrierNode(label) } predicate isBarrier(DataFlow::Node node) { @@ -52,19 +50,18 @@ module UnvalidatedDynamicMethodCallConfig implements DataFlow::StateConfigSig { } predicate isAdditionalFlowStep( - DataFlow::Node src, DataFlow::FlowLabel srclabel, DataFlow::Node dst, - DataFlow::FlowLabel dstlabel + DataFlow::Node src, FlowState srclabel, DataFlow::Node dst, FlowState dstlabel ) { exists(DataFlow::PropRead read | src = read.getPropertyNameExpr().flow() and dst = read and - srclabel.isTaint() and + srclabel = FlowState::taint() and ( - dstlabel instanceof MaybeNonFunction + dstlabel = FlowState::maybeNonFunction() or // a property of `Object.create(null)` cannot come from a prototype not PropertyInjection::isPrototypeLessObject(read.getBase().getALocalSource()) and - dstlabel instanceof MaybeFromProto + dstlabel = FlowState::maybeFromProto() ) and // avoid overlapping results with unsafe dynamic method access query not PropertyInjection::hasUnsafeMethods(read.getBase().getALocalSource()) @@ -74,10 +71,10 @@ module UnvalidatedDynamicMethodCallConfig implements DataFlow::StateConfigSig { src = get.getArgument(0) and dst = get ) and - srclabel.isTaint() and - dstlabel instanceof MaybeNonFunction + srclabel = FlowState::taint() and + dstlabel = FlowState::maybeNonFunction() or - srclabel.isTaint() and + srclabel = FlowState::taint() and TaintTracking::defaultTaintStep(src, dst) and srclabel = dstlabel } @@ -118,6 +115,7 @@ deprecated class Configuration extends TaintTracking::Configuration { DataFlow::Node src, DataFlow::Node dst, DataFlow::FlowLabel srclabel, DataFlow::FlowLabel dstlabel ) { - UnvalidatedDynamicMethodCallConfig::isAdditionalFlowStep(src, srclabel, dst, dstlabel) + UnvalidatedDynamicMethodCallConfig::isAdditionalFlowStep(src, + FlowState::fromFlowLabel(srclabel), dst, FlowState::fromFlowLabel(dstlabel)) } } From a398599bfbd9d594606a4017fe269827fc9f8f4a Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 11:54:11 +0100 Subject: [PATCH 25/37] JS: Rename an experimental query Having the same name as a standard query is just confusing --- .../src/experimental/Security/CWE-094-dataURL/CodeInjection.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.ql b/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.ql index 1e6f7a64aec3..a9b42691c679 100644 --- a/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.ql +++ b/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.ql @@ -1,5 +1,5 @@ /** - * @name Code injection + * @name Code injection from dynamically imported code * @description Interpreting unsanitized user input as code allows a malicious user arbitrary * code execution. * @kind path-problem From d83ddfabaa4eaa9867a8ecd1ad1dcfe1faab6354 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 11:54:24 +0100 Subject: [PATCH 26/37] JS: Migrate an experimental CodeInjection query --- .../Security/CWE-094-dataURL/CodeInjection.ql | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.ql b/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.ql index a9b42691c679..22aa99e7e55c 100644 --- a/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.ql +++ b/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.ql @@ -49,38 +49,43 @@ class WorkerThreads extends DataFlow::Node { } } -class UrlConstructorLabel extends DataFlow::FlowLabel { - UrlConstructorLabel() { this = "UrlConstructorLabel" } -} +newtype TFlowState = + TTaint() or + TUrlConstructor() /** * A taint-tracking configuration for reasoning about code injection vulnerabilities. */ module CodeInjectionConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + class FlowState extends TFlowState { + string toString() { + this = TTaint() and result = "taint" + or + this = TUrlConstructor() and result = "url-constructor" + } + } - predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { - source instanceof ActiveThreatModelSource and label.isTaint() + predicate isSource(DataFlow::Node source, FlowState label) { + source instanceof ActiveThreatModelSource and label = TTaint() } predicate isSink(DataFlow::Node sink) { sink instanceof DynamicImport } - predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - sink instanceof WorkerThreads and label instanceof UrlConstructorLabel + predicate isSink(DataFlow::Node sink, FlowState label) { + sink instanceof WorkerThreads and label = TUrlConstructor() } predicate isBarrier(DataFlow::Node node) { node instanceof Barrier } predicate isAdditionalFlowStep( - DataFlow::Node pred, DataFlow::FlowLabel predlbl, DataFlow::Node succ, - DataFlow::FlowLabel succlbl + DataFlow::Node pred, FlowState predlbl, DataFlow::Node succ, FlowState succlbl ) { exists(DataFlow::NewNode newUrl | succ = newUrl | newUrl = DataFlow::globalVarRef("URL").getAnInstantiation() and pred = newUrl.getArgument(0) ) and - predlbl.isDataOrTaint() and - succlbl instanceof UrlConstructorLabel + predlbl = TTaint() and + succlbl = TUrlConstructor() } } From ebe596f22798a87a487615bcc148dbb28b381d98 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 12:39:50 +0100 Subject: [PATCH 27/37] JS: Migrate CorsPermissiveConfiguration --- ...sPermissiveConfigurationCustomizations.qll | 47 +++++++++++++++++-- .../CorsPermissiveConfigurationQuery.qll | 25 +++++----- 2 files changed, 56 insertions(+), 16 deletions(-) diff --git a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationCustomizations.qll b/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationCustomizations.qll index 103872847a0f..8876373a3d24 100644 --- a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationCustomizations.qll +++ b/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationCustomizations.qll @@ -10,6 +10,45 @@ import Apollo::Apollo /** Module containing sources, sinks, and sanitizers for overly permissive CORS configurations. */ module CorsPermissiveConfiguration { + private newtype TFlowState = + TTaint() or + TTrueOrNull() or + TWildcard() + + /** A flow state to asociate with a tracked value. */ + class FlowState extends TFlowState { + /** Gets a string representation of this flow state. */ + string toString() { + this = TTaint() and result = "taint" + or + this = TTrueOrNull() and result = "true-or-null" + or + this = TWildcard() and result = "wildcard" + } + + deprecated DataFlow::FlowLabel toFlowLabel() { + this = TTaint() and result.isTaint() + or + this = TTrueOrNull() and result instanceof TrueAndNull + or + this = TWildcard() and result instanceof Wildcard + } + } + + /** Predicates for working with flow states. */ + module FlowState { + deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } + + /** A tainted value. */ + FlowState taint() { result = TTaint() } + + /** A `true` or `null` value. */ + FlowState trueOrNull() { result = TTrueOrNull() } + + /** A `"*"` value. */ + FlowState wildcard() { result = TWildcard() } + } + /** * A data flow source for permissive CORS configuration. */ @@ -38,18 +77,18 @@ module CorsPermissiveConfiguration { } /** A flow label representing `true` and `null` values. */ - abstract class TrueAndNull extends DataFlow::FlowLabel { + abstract deprecated class TrueAndNull extends DataFlow::FlowLabel { TrueAndNull() { this = "TrueAndNull" } } - TrueAndNull truenullLabel() { any() } + deprecated TrueAndNull truenullLabel() { any() } /** A flow label representing `*` value. */ - abstract class Wildcard extends DataFlow::FlowLabel { + abstract deprecated class Wildcard extends DataFlow::FlowLabel { Wildcard() { this = "Wildcard" } } - Wildcard wildcardLabel() { any() } + deprecated Wildcard wildcardLabel() { any() } /** An overly permissive value for `origin` (Apollo) */ class TrueNullValue extends Source { diff --git a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationQuery.qll b/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationQuery.qll index ea17de322ac6..bddc732dea21 100644 --- a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationQuery.qll +++ b/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationQuery.qll @@ -10,25 +10,26 @@ import javascript import CorsPermissiveConfigurationCustomizations::CorsPermissiveConfiguration +private import CorsPermissiveConfigurationCustomizations::CorsPermissiveConfiguration as CorsPermissiveConfiguration /** * A data flow configuration for overly permissive CORS configuration. */ module CorsPermissiveConfigurationConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + class FlowState = CorsPermissiveConfiguration::FlowState; - predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { - source instanceof TrueNullValue and label = truenullLabel() + predicate isSource(DataFlow::Node source, FlowState state) { + source instanceof TrueNullValue and state = FlowState::trueOrNull() or - source instanceof WildcardValue and label = wildcardLabel() + source instanceof WildcardValue and state = FlowState::wildcard() or - source instanceof RemoteFlowSource and label = DataFlow::FlowLabel::taint() + source instanceof RemoteFlowSource and state = FlowState::taint() } - predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - sink instanceof CorsApolloServer and label = [DataFlow::FlowLabel::taint(), truenullLabel()] + predicate isSink(DataFlow::Node sink, FlowState state) { + sink instanceof CorsApolloServer and state = [FlowState::taint(), FlowState::trueOrNull()] or - sink instanceof ExpressCors and label = [DataFlow::FlowLabel::taint(), wildcardLabel()] + sink instanceof ExpressCors and state = [FlowState::taint(), FlowState::wildcard()] } predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } @@ -44,11 +45,11 @@ deprecated class Configuration extends TaintTracking::Configuration { Configuration() { this = "CorsPermissiveConfiguration" } override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { - CorsPermissiveConfigurationConfig::isSource(source, label) + CorsPermissiveConfigurationConfig::isSource(source, FlowState::fromFlowLabel(label)) } override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - CorsPermissiveConfigurationConfig::isSink(sink, label) + CorsPermissiveConfigurationConfig::isSink(sink, FlowState::fromFlowLabel(label)) } override predicate isSanitizer(DataFlow::Node node) { @@ -57,10 +58,10 @@ deprecated class Configuration extends TaintTracking::Configuration { } } -private class WildcardActivated extends DataFlow::FlowLabel, Wildcard { +deprecated private class WildcardActivated extends DataFlow::FlowLabel, Wildcard { WildcardActivated() { this = this } } -private class TrueAndNullActivated extends DataFlow::FlowLabel, TrueAndNull { +deprecated private class TrueAndNullActivated extends DataFlow::FlowLabel, TrueAndNull { TrueAndNullActivated() { this = this } } From 73af3f35369ad71b965c7957f4c88df5b959a93a Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 12:55:05 +0100 Subject: [PATCH 28/37] JS: Migrate PrototypePollutingFunction --- .../CWE-915/PrototypePollutingFunction.ql | 93 +++++++++---------- 1 file changed, 43 insertions(+), 50 deletions(-) diff --git a/javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql b/javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql index 5df916c9d2ba..ba7c6d177cbb 100644 --- a/javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql +++ b/javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql @@ -192,14 +192,6 @@ string unsafePropName() { result = "constructor" } -/** - * A flow label representing an unsafe property name, or an object obtained - * by using such a property in a dynamic read. - */ -class UnsafePropLabel extends DataFlow::FlowLabel { - UnsafePropLabel() { this = unsafePropName() } -} - /** * Tracks data from property enumerations to dynamic property writes. * @@ -233,10 +225,12 @@ class UnsafePropLabel extends DataFlow::FlowLabel { * a standalone configuration like in most path queries. */ module PropNameTrackingConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + class FlowState extends string { + FlowState() { this = unsafePropName() } + } - predicate isSource(DataFlow::Node node, DataFlow::FlowLabel label) { - label instanceof UnsafePropLabel and + predicate isSource(DataFlow::Node node, FlowState state) { + exists(state) and ( isPollutedPropNameSource(node) or @@ -244,8 +238,8 @@ module PropNameTrackingConfig implements DataFlow::StateConfigSig { ) } - predicate isSink(DataFlow::Node node, DataFlow::FlowLabel label) { - label instanceof UnsafePropLabel and + predicate isSink(DataFlow::Node node, FlowState state) { + exists(state) and ( dynamicPropWrite(node, _, _) or dynamicPropWrite(_, node, _) or @@ -253,29 +247,28 @@ module PropNameTrackingConfig implements DataFlow::StateConfigSig { ) } - predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel label) { - node = DataFlow::MakeLabeledBarrierGuard::getABarrierNode(label) + predicate isBarrier(DataFlow::Node node, FlowState state) { + node = DataFlow::MakeStateBarrierGuard::getABarrierNode(state) } predicate isAdditionalFlowStep( - DataFlow::Node pred, DataFlow::FlowLabel predlbl, DataFlow::Node succ, - DataFlow::FlowLabel succlbl + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - predlbl instanceof UnsafePropLabel and - succlbl = predlbl and + exists(state1) and + state2 = state1 and ( // Step through `p -> x[p]` exists(DataFlow::PropRead read | - pred = read.getPropertyNameExpr().flow() and + node1 = read.getPropertyNameExpr().flow() and not read.(DynamicPropRead).hasDominatingAssignment() and - succ = read + node2 = read ) or // Step through `x -> x[p]` exists(DynamicPropRead read | not read.hasDominatingAssignment() and - pred = read.getBase() and - succ = read + node1 = read.getBase() and + node2 = read ) ) } @@ -286,6 +279,8 @@ module PropNameTrackingConfig implements DataFlow::StateConfigSig { } } +class FlowState = PropNameTrackingConfig::FlowState; + module PropNameTracking = DataFlow::GlobalWithState; /** @@ -298,9 +293,9 @@ abstract class BarrierGuard extends DataFlow::Node { predicate blocksExpr(boolean outcome, Expr e) { none() } /** - * Holds if this node acts as a barrier for `label`, blocking further flow from `e` if `this` evaluates to `outcome`. + * Holds if this node acts as a barrier for `state`, blocking further flow from `e` if `this` evaluates to `outcome`. */ - predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { none() } + predicate blocksExpr(boolean outcome, Expr e, FlowState state) { none() } } /** @@ -315,10 +310,10 @@ class DenyListEqualityGuard extends BarrierGuard, DataFlow::ValueNode { propName = unsafePropName() } - override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { e = astNode.getAnOperand() and outcome = astNode.getPolarity().booleanNot() and - label = propName + state = propName } } @@ -333,10 +328,9 @@ class AllowListEqualityGuard extends BarrierGuard, DataFlow::ValueNode { astNode.getAnOperand() instanceof Literal } - override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { + override predicate blocksExpr(boolean outcome, Expr e) { e = astNode.getAnOperand() and - outcome = astNode.getPolarity() and - label instanceof UnsafePropLabel + outcome = astNode.getPolarity() } } @@ -382,24 +376,24 @@ class InExprGuard extends BarrierGuard, DataFlow::ValueNode { /** * A sanitizer guard for `instanceof` expressions. * - * `Object.prototype instanceof X` is never true, so this blocks the `__proto__` label. + * `Object.prototype instanceof X` is never true, so this blocks the `__proto__` state. * * It is still possible to get to `Function.prototype` through `constructor.constructor.prototype` - * so we do not block the `constructor` label. + * so we do not block the `constructor` state. */ class InstanceOfGuard extends BarrierGuard, DataFlow::ValueNode { override InstanceOfExpr astNode; - override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { - e = astNode.getLeftOperand() and outcome = true and label = "__proto__" + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { + e = astNode.getLeftOperand() and outcome = true and state = "__proto__" } } /** * Sanitizer guard of form `typeof x === "object"` or `typeof x === "function"`. * - * The former blocks the `constructor` label as that payload must pass through a function, - * and the latter blocks the `__proto__` label as that only passes through objects. + * The former blocks the `constructor` state as that payload must pass through a function, + * and the latter blocks the `__proto__` state as that only passes through objects. */ class TypeofGuard extends BarrierGuard, DataFlow::ValueNode { override EqualityTest astNode; @@ -408,15 +402,15 @@ class TypeofGuard extends BarrierGuard, DataFlow::ValueNode { TypeofGuard() { TaintTracking::isTypeofGuard(astNode, operand, tag) } - override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { e = operand and outcome = astNode.getPolarity() and ( tag = "object" and - label = "constructor" + state = "constructor" or tag = "function" and - label = "__proto__" + state = "__proto__" ) or e = operand and @@ -425,10 +419,10 @@ class TypeofGuard extends BarrierGuard, DataFlow::ValueNode { // If something is not an object, sanitize object, as both must end // in non-function prototype object. tag = "object" and - label instanceof UnsafePropLabel + exists(state) or tag = "function" and - label = "constructor" + state = "constructor" ) } } @@ -437,19 +431,19 @@ class TypeofGuard extends BarrierGuard, DataFlow::ValueNode { * A check of form `["__proto__"].includes(x)` or similar. */ class DenyListInclusionGuard extends BarrierGuard, InclusionTest { - UnsafePropLabel label; + string blockedProp; DenyListInclusionGuard() { exists(DataFlow::ArrayCreationNode array | - array.getAnElement().getStringValue() = label and + array.getAnElement().getStringValue() = blockedProp and array.flowsTo(this.getContainerNode()) ) } - override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel lbl) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { outcome = this.getPolarity().booleanNot() and e = this.getContainedNode().asExpr() and - label = lbl + blockedProp = state } } @@ -464,9 +458,8 @@ class AllowListInclusionGuard extends BarrierGuard { not this = any(MembershipCandidate::ObjectPropertyNameMembershipCandidate c).getTest() // handled with more precision in `HasOwnPropertyGuard` } - override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel lbl) { - this.(TaintTracking::AdditionalBarrierGuard).blocksExpr(outcome, e) and - lbl instanceof UnsafePropLabel + override predicate blocksExpr(boolean outcome, Expr e) { + this.(TaintTracking::AdditionalBarrierGuard).blocksExpr(outcome, e) } } @@ -482,10 +475,10 @@ class IsPlainObjectGuard extends BarrierGuard, DataFlow::CallNode { ) } - override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel lbl) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { e = this.getArgument(0).asExpr() and outcome = true and - lbl = "constructor" + state = "constructor" } } From 69b361ae704ecdd76d4421f7a8f94a2f91e202ea Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 12:55:20 +0100 Subject: [PATCH 29/37] JS: Migrate a test to use flow state --- .../InterProceduralFlow/tests.ql | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/tests.ql b/javascript/ql/test/library-tests/InterProceduralFlow/tests.ql index e20ec8ff6d4e..a32e996b6b56 100644 --- a/javascript/ql/test/library-tests/InterProceduralFlow/tests.ql +++ b/javascript/ql/test/library-tests/InterProceduralFlow/tests.ql @@ -3,33 +3,42 @@ import DataFlowConfig query predicate dataFlow(DataFlow::Node src, DataFlow::Node snk) { TestFlow::flow(src, snk) } -class Parity extends DataFlow::FlowLabel { - Parity() { this = "even" or this = "odd" } - - Parity flip() { result != this } -} - module FlowLabelConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowLabel; + private newtype TFlowState = + TEven() or + TOdd() + + class FlowState extends TFlowState { + string toString() { + this = TEven() and result = "even" + or + this = TOdd() and result = "odd" + } + + FlowState flip() { + this = TEven() and result = TOdd() + or + this = TOdd() and result = TEven() + } + } - predicate isSource(DataFlow::Node nd, DataFlow::FlowLabel lbl) { + predicate isSource(DataFlow::Node nd, FlowState state) { nd.(DataFlow::CallNode).getCalleeName() = "source" and - lbl = "even" + state = TEven() } - predicate isSink(DataFlow::Node nd, DataFlow::FlowLabel lbl) { + predicate isSink(DataFlow::Node nd, FlowState state) { nd = any(DataFlow::CallNode c | c.getCalleeName() = "sink").getAnArgument() and - lbl = "even" + state = TEven() } predicate isAdditionalFlowStep( - DataFlow::Node pred, DataFlow::FlowLabel predLabel, DataFlow::Node succ, - DataFlow::FlowLabel succLabel + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - exists(DataFlow::CallNode c | c = succ | + exists(DataFlow::CallNode c | c = node2 | c.getCalleeName() = "inc" and - pred = c.getAnArgument() and - succLabel = predLabel.(Parity).flip() + node1 = c.getAnArgument() and + state2 = state1.flip() ) } } From d993c888b144764fef23dbefe0403856000281a0 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 13:15:11 +0100 Subject: [PATCH 30/37] JS: Deprecate the FlowLabel class --- .../javascript/dataflow/AdditionalFlowSteps.qll | 16 ++++++++++++---- .../semmle/javascript/dataflow/Configuration.qll | 7 +++++-- .../dataflow/internal/BarrierGuards.qll | 14 ++++++++------ .../javascript/dataflow/internal/FlowSteps.qll | 8 ++++---- 4 files changed, 29 insertions(+), 16 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/AdditionalFlowSteps.qll b/javascript/ql/lib/semmle/javascript/dataflow/AdditionalFlowSteps.qll index 49b6d11bbc9e..d0deff8788ca 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/AdditionalFlowSteps.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/AdditionalFlowSteps.qll @@ -147,10 +147,12 @@ class LegacyFlowStep extends Unit { predicate step(DataFlow::Node pred, DataFlow::Node succ) { none() } /** + * DEPRECATED. The `FlowLabel` class and steps involving flow labels are no longer used by any queries. + * * Holds if `pred` → `succ` should be considered a data flow edge * transforming values with label `predlbl` to have label `succlbl`. */ - predicate step( + deprecated predicate step( DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel predlbl, DataFlow::FlowLabel succlbl ) { @@ -199,11 +201,13 @@ module LegacyFlowStep { } /** + * DEPRECATED. The `FlowLabel` class and steps involving flow labels are no longer used by any queries. + * * Holds if `pred` → `succ` should be considered a data flow edge * transforming values with label `predlbl` to have label `succlbl`. */ cached - predicate step( + deprecated predicate step( DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel predlbl, DataFlow::FlowLabel succlbl ) { @@ -273,10 +277,12 @@ class SharedFlowStep extends Unit { predicate step(DataFlow::Node pred, DataFlow::Node succ) { none() } /** + * DEPRECATED. The `FlowLabel` class and steps involving flow labels are no longer used by any queries. + * * Holds if `pred` → `succ` should be considered a data flow edge * transforming values with label `predlbl` to have label `succlbl`. */ - predicate step( + deprecated predicate step( DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel predlbl, DataFlow::FlowLabel succlbl ) { @@ -353,10 +359,12 @@ module SharedFlowStep { // The following are aliases for old step predicates that have no corresponding predicate in AdditionalFlowStep /** + * DEPRECATED. The `FlowLabel` class and steps involving flow labels are no longer used by any queries. + * * Holds if `pred` → `succ` should be considered a data flow edge * transforming values with label `predlbl` to have label `succlbl`. */ - predicate step( + deprecated predicate step( DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel predlbl, DataFlow::FlowLabel succlbl ) { diff --git a/javascript/ql/lib/semmle/javascript/dataflow/Configuration.qll b/javascript/ql/lib/semmle/javascript/dataflow/Configuration.qll index 74c445582d8f..bb032596475e 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/Configuration.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/Configuration.qll @@ -295,6 +295,9 @@ deprecated private predicate isBarrierGuardInternal( } /** + * DEPRECATED. Use a query-specific `FlowState` class instead. + * See [guide on using flow state](https://codeql.github.com/docs/codeql-language-guides/using-flow-labels-for-precise-data-flow-analysis) for more details. + * * A label describing the kind of information tracked by a flow configuration. * * There are two standard labels "data" and "taint". @@ -303,7 +306,7 @@ deprecated private predicate isBarrierGuardInternal( * - "taint" additionally permits flow through transformations such as string operations, * and is the default flow source for a `TaintTracking::Configuration`. */ -abstract class FlowLabel extends string { +abstract deprecated class FlowLabel extends string { bindingset[this] FlowLabel() { any() } @@ -341,7 +344,7 @@ deprecated class StandardFlowLabel extends FlowLabel { StandardFlowLabel() { this = "data" or this = "taint" } } -module FlowLabel { +deprecated module FlowLabel { /** * Gets the standard flow label for describing values that directly originate from a flow source. */ diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/BarrierGuards.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/BarrierGuards.qll index 50e5d48278c1..d02728ef551c 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/BarrierGuards.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/BarrierGuards.qll @@ -36,17 +36,19 @@ module MakeBarrierGuard { } } -private signature class LabeledBarrierGuardSig extends DataFlow::Node { - /** - * Holds if this node acts as a barrier for `label`, blocking further flow from `e` if `this` evaluates to `outcome`. - */ - predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label); +deprecated private module DeprecationWrapper { + signature class LabeledBarrierGuardSig extends DataFlow::Node { + /** + * Holds if this node acts as a barrier for `label`, blocking further flow from `e` if `this` evaluates to `outcome`. + */ + predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label); + } } /** * Converts a barrier guard class to a set of nodes to include in an implementation of `isBarrier(node, label)`. */ -module MakeLabeledBarrierGuard { +deprecated module MakeLabeledBarrierGuard { final private class FinalBaseGuard = BaseGuard; private class Adapter extends FinalBaseGuard { diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/FlowSteps.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/FlowSteps.qll index b6152bcf3c38..ff2cbb3d4be1 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/FlowSteps.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/FlowSteps.qll @@ -545,9 +545,9 @@ class Boolean extends boolean { /** * A summary of an inter-procedural data flow path. */ -newtype TPathSummary = +deprecated newtype TPathSummary = /** A summary of an inter-procedural data flow path. */ - MkPathSummary(Boolean hasReturn, Boolean hasCall, FlowLabel start, FlowLabel end) + deprecated MkPathSummary(Boolean hasReturn, Boolean hasCall, FlowLabel start, FlowLabel end) /** * A summary of an inter-procedural data flow path. @@ -560,7 +560,7 @@ newtype TPathSummary = * We only want to build properly matched call/return sequences, so if a path has both * call steps and return steps, all return steps must precede all call steps. */ -class PathSummary extends TPathSummary { +deprecated class PathSummary extends TPathSummary { Boolean hasReturn; Boolean hasCall; FlowLabel start; @@ -634,7 +634,7 @@ class PathSummary extends TPathSummary { } } -module PathSummary { +deprecated module PathSummary { /** * Gets a summary describing a path without any calls or returns. */ From ac6da6c2b12b5e6328ebbe486b445d05b98ea6c5 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 13:27:37 +0100 Subject: [PATCH 31/37] JS: Add some missing qldoc --- javascript/ql/lib/semmle/javascript/security/TaintedObject.qll | 3 +++ .../security/dataflow/ExceptionXssCustomizations.qll | 2 ++ .../dataflow/HardcodedDataInterpretedAsCodeCustomizations.qll | 2 ++ .../security/dataflow/InsecureDownloadCustomizations.qll | 2 ++ .../dataflow/PrototypePollutingAssignmentCustomizations.qll | 2 ++ .../dataflow/UnsafeDynamicMethodAccessCustomizations.qll | 2 ++ .../dataflow/UnvalidatedDynamicMethodCallCustomizations.qll | 2 ++ 7 files changed, 15 insertions(+) diff --git a/javascript/ql/lib/semmle/javascript/security/TaintedObject.qll b/javascript/ql/lib/semmle/javascript/security/TaintedObject.qll index 97cef346ce48..ca903fe54951 100644 --- a/javascript/ql/lib/semmle/javascript/security/TaintedObject.qll +++ b/javascript/ql/lib/semmle/javascript/security/TaintedObject.qll @@ -26,6 +26,9 @@ module TaintedObject { ConcreteTaintedObjectLabel() { this = this } } + /** + * DEPRECATED. Use `isAdditionalFlowStep(node1, state1, node2, state2)` instead. + */ deprecated predicate step(Node src, Node trg, FlowLabel inlbl, FlowLabel outlbl) { isAdditionalFlowStep(src, FlowState::fromFlowLabel(inlbl), trg, FlowState::fromFlowLabel(outlbl)) } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssCustomizations.qll index 9c1492be2811..70281110a5fb 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssCustomizations.qll @@ -27,6 +27,7 @@ module ExceptionXss { this = TNotYetThrown() and result = "not-yet-thrown" } + /** Gets the corresponding flow label. */ deprecated DataFlow::FlowLabel toFlowLabel() { this = TThrown() and result.isTaint() or @@ -36,6 +37,7 @@ module ExceptionXss { /** Predicates for working with flow states. */ module FlowState { + /** Gets the flow state corresponding to `label`. */ deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } /** A tainted value originating from a thrown and caught exception. */ diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeCustomizations.qll index 76549122b868..9ec6f2d5b4ad 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeCustomizations.qll @@ -21,6 +21,7 @@ module HardcodedDataInterpretedAsCode { this = TModified() and result = "modified" } + /** Gets the corresponding flow label. */ deprecated DataFlow::FlowLabel toFlowLabel() { this = TUnmodified() and result.isData() or @@ -30,6 +31,7 @@ module HardcodedDataInterpretedAsCode { /** Predicates for working with flow states. */ module FlowState { + /** Gets the flow state corresponding to `label`. */ deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } /** An unmodified value originating from a string constant. */ diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadCustomizations.qll index d8e66e65122b..d4488840bb4f 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadCustomizations.qll @@ -23,6 +23,7 @@ module InsecureDownload { this = TInsecureUrl() and result = "insecure-url" } + /** Gets the corresponding flow label. */ deprecated DataFlow::FlowLabel toFlowLabel() { this = TSensitiveInsecureUrl() and result instanceof Label::SensitiveInsecureUrl or @@ -32,6 +33,7 @@ module InsecureDownload { /** Predicates for working with flow states. */ module FlowState { + /** Gets the flow state corresponding to `label`. */ deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } /** diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentCustomizations.qll index a196cac42178..bb2f9739501c 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentCustomizations.qll @@ -23,6 +23,7 @@ module PrototypePollutingAssignment { this = TObjectPrototype() and result = "object-prototype" } + /** Gets the corresponding flow label. */ deprecated DataFlow::FlowLabel toFlowLabel() { this = TTaint() and result.isTaint() or @@ -32,6 +33,7 @@ module PrototypePollutingAssignment { /** Predicates for working with flow states. */ module FlowState { + /** Gets the flow state corresponding to `label`. */ deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } /** A tainted value. */ diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessCustomizations.qll index 711157a8d3e7..756efb5f3fb6 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessCustomizations.qll @@ -23,6 +23,7 @@ module UnsafeDynamicMethodAccess { this = TUnsafeFunction() and result = "unsafe-function" } + /** Gets the corresponding flow label. */ deprecated DataFlow::FlowLabel toFlowLabel() { this = TTaint() and result.isTaint() or @@ -32,6 +33,7 @@ module UnsafeDynamicMethodAccess { /** Predicates for working with flow states. */ module FlowState { + /** Gets the flow state corresponding to `label`. */ deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } /** A tainted value. */ diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll index 19240f94a110..f148ba2c96bb 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll @@ -26,6 +26,7 @@ module UnvalidatedDynamicMethodCall { this = TMaybeFromProto() and result = "maybe-from-proto" } + /** Gets the corresponding flow label. */ deprecated DataFlow::FlowLabel toFlowLabel() { this = TTaint() and result.isTaint() or @@ -37,6 +38,7 @@ module UnvalidatedDynamicMethodCall { /** Predicates for working with flow states. */ module FlowState { + /** Gets the flow state corresponding to `label`. */ deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } /** A tainted value. */ From 079294e55f7f5724fa0845094bc64e66002e9075 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 14:57:05 +0100 Subject: [PATCH 32/37] JS: Mass rename to node1,state1,node2,state2 naming convention --- .../dataflow/BackendIdor/BackendIdor.ql | 4 +- .../InformationDisclosure.ql | 8 +-- .../javascript/security/TaintedObject.qll | 46 ++++++------- .../TaintedUrlSuffixCustomizations.qll | 30 ++++----- .../dataflow/BuildArtifactLeakQuery.qll | 4 +- .../dataflow/CleartextLoggingQuery.qll | 4 +- .../ClientSideRequestForgeryQuery.qll | 4 +- .../dataflow/ClientSideUrlRedirectQuery.qll | 2 +- .../dataflow/ConditionalBypassQuery.qll | 4 +- .../security/dataflow/ExceptionXssQuery.qll | 10 +-- .../dataflow/HardcodedCredentialsQuery.qll | 36 +++++----- .../HardcodedDataInterpretedAsCodeQuery.qll | 6 +- ...completeHtmlAttributeSanitizationQuery.qll | 12 ++-- .../dataflow/InsecureRandomnessQuery.qll | 6 +- .../dataflow/LoopBoundInjectionQuery.qll | 4 +- .../PrototypePollutingAssignmentQuery.qll | 66 +++++++++---------- .../dataflow/PrototypePollutionQuery.qll | 4 +- .../security/dataflow/RequestForgeryQuery.qll | 4 +- .../dataflow/ResourceExhaustionQuery.qll | 4 +- .../SecondOrderCommandInjectionQuery.qll | 10 +-- .../dataflow/ServerSideUrlRedirectQuery.qll | 6 +- .../security/dataflow/SqlInjectionQuery.qll | 10 +-- .../dataflow/TemplateObjectInjectionQuery.qll | 10 +-- .../dataflow/UnsafeCodeConstruction.qll | 4 +- .../UnsafeDynamicMethodAccessQuery.qll | 38 +++++------ .../dataflow/UnsafeHtmlConstructionQuery.qll | 16 ++--- .../dataflow/UnsafeJQueryPluginQuery.qll | 4 +- .../UnsafeShellCommandConstructionQuery.qll | 2 +- .../UnvalidatedDynamicMethodCallQuery.qll | 40 +++++------ .../security/dataflow/XssThroughDomQuery.qll | 6 +- .../security/regexp/PolynomialReDoSQuery.qll | 2 +- .../Security/CWE-094-dataURL/CodeInjection.ql | 18 ++--- .../CWE-099/EnvValueAndKeyInjection.ql | 10 +-- .../DecompressionBombs.ql | 4 +- 34 files changed, 220 insertions(+), 218 deletions(-) diff --git a/javascript/ql/examples/queries/dataflow/BackendIdor/BackendIdor.ql b/javascript/ql/examples/queries/dataflow/BackendIdor/BackendIdor.ql index 92f5dad50c73..ac8d7206c667 100644 --- a/javascript/ql/examples/queries/dataflow/BackendIdor/BackendIdor.ql +++ b/javascript/ql/examples/queries/dataflow/BackendIdor/BackendIdor.ql @@ -18,9 +18,9 @@ module IdorTaintConfig implements DataFlow::ConfigSig { predicate isSink(DataFlow::Node node) { exists(ClientRequest req | node = req.getADataNode()) } - predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { // Step from x -> { userId: x } - succ.(DataFlow::SourceNode).getAPropertyWrite("userId").getRhs() = pred + node2.(DataFlow::SourceNode).getAPropertyWrite("userId").getRhs() = node1 } predicate isBarrier(DataFlow::Node node) { diff --git a/javascript/ql/examples/queries/dataflow/InformationDisclosure/InformationDisclosure.ql b/javascript/ql/examples/queries/dataflow/InformationDisclosure/InformationDisclosure.ql index 64a1c6c801f3..e23f4aa53f9b 100644 --- a/javascript/ql/examples/queries/dataflow/InformationDisclosure/InformationDisclosure.ql +++ b/javascript/ql/examples/queries/dataflow/InformationDisclosure/InformationDisclosure.ql @@ -37,16 +37,16 @@ module AuthKeyTrackingConfig implements DataFlow::ConfigSig { ) } - predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { // Step into objects: x -> { f: x } - succ.(DataFlow::SourceNode).getAPropertyWrite().getRhs() = pred + node2.(DataFlow::SourceNode).getAPropertyWrite().getRhs() = node1 or // Step through JSON serialization: x -> JSON.stringify(x) // Note: TaintTracking::Configuration includes this step by default, but not DataFlow::Configuration exists(DataFlow::CallNode call | call = DataFlow::globalVarRef("JSON").getAMethodCall("stringify") and - pred = call.getArgument(0) and - succ = call + node1 = call.getArgument(0) and + node2 = call ) } } diff --git a/javascript/ql/lib/semmle/javascript/security/TaintedObject.qll b/javascript/ql/lib/semmle/javascript/security/TaintedObject.qll index ca903fe54951..a300291ae9cd 100644 --- a/javascript/ql/lib/semmle/javascript/security/TaintedObject.qll +++ b/javascript/ql/lib/semmle/javascript/security/TaintedObject.qll @@ -36,42 +36,42 @@ module TaintedObject { /** * Holds for the flows steps that are relevant for tracking user-controlled JSON objects. */ - predicate isAdditionalFlowStep(Node src, FlowState inlbl, Node trg, FlowState outlbl) { + predicate isAdditionalFlowStep(Node node1, FlowState state1, Node node2, FlowState state2) { // JSON parsers map tainted inputs to tainted JSON - inlbl.isTaint() and - outlbl.isTaintedObject() and + state1.isTaint() and + state2.isTaintedObject() and exists(JsonParserCall parse | - src = parse.getInput() and - trg = parse.getOutput() + node1 = parse.getInput() and + node2 = parse.getOutput() ) or // Property reads preserve deep object taint. - inlbl.isTaintedObject() and - outlbl.isTaintedObject() and - trg.(PropRead).getBase() = src + state1.isTaintedObject() and + state2.isTaintedObject() and + node2.(PropRead).getBase() = node1 or // Property projection preserves deep object taint - inlbl.isTaintedObject() and - outlbl.isTaintedObject() and - trg.(PropertyProjection).getObject() = src + state1.isTaintedObject() and + state2.isTaintedObject() and + node2.(PropertyProjection).getObject() = node1 or // Extending objects preserves deep object taint - inlbl.isTaintedObject() and - outlbl.isTaintedObject() and + state1.isTaintedObject() and + state2.isTaintedObject() and exists(ExtendCall call | - src = call.getAnOperand() and - trg = call + node1 = call.getAnOperand() and + node2 = call or - src = call.getASourceOperand() and - trg = call.getDestinationOperand().getALocalSource() + node1 = call.getASourceOperand() and + node2 = call.getDestinationOperand().getALocalSource() ) or // Spreading into an object preserves deep object taint: `p -> { ...p }` - inlbl.isTaintedObject() and - outlbl.isTaintedObject() and + state1.isTaintedObject() and + state2.isTaintedObject() and exists(ObjectLiteralNode obj | - src = obj.getASpreadProperty() and - trg = obj + node1 = obj.getASpreadProperty() and + node2 = obj ) } @@ -96,8 +96,8 @@ module TaintedObject { /** Holds if this node blocks flow through `e`, provided it evaluates to `outcome`. */ predicate blocksExpr(boolean outcome, Expr e) { none() } - /** Holds if this node blocks flow of `label` through `e`, provided it evaluates to `outcome`. */ - predicate blocksExpr(boolean outcome, Expr e, FlowState label) { none() } + /** Holds if this node blocks flow of `state` through `e`, provided it evaluates to `outcome`. */ + predicate blocksExpr(boolean outcome, Expr e, FlowState state) { none() } /** DEPRECATED. Use `blocksExpr` instead. */ deprecated predicate sanitizes(boolean outcome, Expr e, FlowLabel label) { diff --git a/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll index 46fdc602d6d6..ee6cf5da6d94 100644 --- a/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll @@ -66,11 +66,11 @@ module TaintedUrlSuffix { } /** - * Holds if there is a flow step `src -> dst` involving the URL suffix flow state. + * Holds if there is a flow step `node1 -> node2` involving the URL suffix flow state. * * This handles steps through string operations, promises, URL parsers, and URL accessors. */ - predicate isAdditionalFlowStep(Node src, FlowState srclbl, Node dst, FlowState dstlbl) { + predicate isAdditionalFlowStep(Node node1, FlowState state1, Node node2, FlowState state2) { // Transition from tainted-url-suffix to general taint when entering the second array element // of a split('#') or split('?') array. // @@ -79,17 +79,17 @@ module TaintedUrlSuffix { // Technically we should also preverse tainted-url-suffix when entering the first array element of such // a split, but this mostly leads to FPs since we currently don't track if the taint has been through URI-decoding. // (The query/fragment parts are often URI-decoded in practice, but not the other URL parts are not) - srclbl.isTaintedUrlSuffix() and - dstlbl.isTaint() and - DataFlowPrivate::optionalStep(src, "split-url-suffix-post", dst) + state1.isTaintedUrlSuffix() and + state2.isTaint() and + DataFlowPrivate::optionalStep(node1, "split-url-suffix-post", node2) or // Transition from URL suffix to full taint when extracting the query/fragment part. - srclbl.isTaintedUrlSuffix() and - dstlbl.isTaint() and + state1.isTaintedUrlSuffix() and + state2.isTaint() and ( exists(MethodCallNode call, string name | - src = call.getReceiver() and - dst = call and + node1 = call.getReceiver() and + node2 = call and name = call.getMethodName() | // Substring that is not a prefix @@ -118,8 +118,8 @@ module TaintedUrlSuffix { ) or exists(PropRead read | - src = read.getBase() and - dst = read and + node1 = read.getBase() and + node2 = read and // Unlike the `search` property, the `query` property from `url.parse` does not include the `?`. read.getPropertyName() = "query" ) @@ -127,13 +127,13 @@ module TaintedUrlSuffix { exists(MethodCallNode call, DataFlow::RegExpCreationNode re | ( call = re.getAMethodCall("exec") and - src = call.getArgument(0) and - dst = call + node1 = call.getArgument(0) and + node2 = call or call.getMethodName() = ["match", "matchAll"] and re.flowsTo(call.getArgument(0)) and - src = call.getReceiver() and - dst = call + node1 = call.getReceiver() and + node2 = call ) | captureAfterSuffixIndicator(re.getRoot().getAChild*()) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/BuildArtifactLeakQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/BuildArtifactLeakQuery.qll index 0e010e35eebc..5ccaeea6ad63 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/BuildArtifactLeakQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/BuildArtifactLeakQuery.qll @@ -21,8 +21,8 @@ module BuildArtifactLeakConfig implements DataFlow::ConfigSig { predicate isBarrier(DataFlow::Node node) { node instanceof CleartextLogging::Barrier } - predicate isAdditionalFlowStep(DataFlow::Node src, DataFlow::Node trg) { - CleartextLogging::isAdditionalTaintStep(src, trg) + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + CleartextLogging::isAdditionalTaintStep(node1, node2) } predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet contents) { diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/CleartextLoggingQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/CleartextLoggingQuery.qll index 2d222be12141..9bb2ffa0a6a7 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/CleartextLoggingQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/CleartextLoggingQuery.qll @@ -32,8 +32,8 @@ module CleartextLoggingConfig implements DataFlow::ConfigSig { isSource(node) } - predicate isAdditionalFlowStep(DataFlow::Node src, DataFlow::Node trg) { - CleartextLogging::isAdditionalTaintStep(src, trg) + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + CleartextLogging::isAdditionalTaintStep(node1, node2) } predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet contents) { diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideRequestForgeryQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideRequestForgeryQuery.qll index c3856e5bcd2e..d26fe2d50e85 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideRequestForgeryQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideRequestForgeryQuery.qll @@ -28,8 +28,8 @@ module ClientSideRequestForgeryConfig implements DataFlow::ConfigSig { predicate isBarrierOut(DataFlow::Node node) { sanitizingPrefixEdge(node, _) } - predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { - isAdditionalRequestForgeryStep(pred, succ) + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + isAdditionalRequestForgeryStep(node1, node2) } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectQuery.qll index 0fdffc5e01cf..bc0e1354757e 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectQuery.qll @@ -41,7 +41,7 @@ module ClientSideUrlRedirectConfig implements DataFlow::StateConfigSig { predicate isBarrierOut(DataFlow::Node node) { hostnameSanitizingPrefixEdge(node, _) } - predicate isBarrierOut(DataFlow::Node node, FlowState label) { isSink(node, label) } + predicate isBarrierOut(DataFlow::Node node, FlowState state) { isSink(node, state) } predicate isAdditionalFlowStep( DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ConditionalBypassQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ConditionalBypassQuery.qll index 6482b09a754e..8db7c27b5f73 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ConditionalBypassQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ConditionalBypassQuery.qll @@ -20,9 +20,9 @@ module ConditionalBypassConfig implements DataFlow::ConfigSig { predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } - predicate isAdditionalFlowStep(DataFlow::Node src, DataFlow::Node dst) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { // comparing a tainted expression against a constant gives a tainted result - dst.asExpr().(Comparison).hasOperands(src.asExpr(), any(ConstantExpr c)) + node2.asExpr().(Comparison).hasOperands(node1.asExpr(), any(ConstantExpr c)) } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssQuery.qll index 47b65007a5a5..71603d38ecd6 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssQuery.qll @@ -146,12 +146,12 @@ module ExceptionXssConfig implements DataFlow::StateConfigSig { } predicate isAdditionalFlowStep( - DataFlow::Node pred, FlowState inlbl, DataFlow::Node succ, FlowState outlbl + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - inlbl = FlowState::notYetThrown() and - outlbl = [FlowState::thrown(), FlowState::notYetThrown()] and - canThrowSensitiveInformation(pred) and - succ = getExceptionTarget(pred) + state1 = FlowState::notYetThrown() and + state2 = [FlowState::thrown(), FlowState::notYetThrown()] and + canThrowSensitiveInformation(node1) and + node2 = getExceptionTarget(node1) } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedCredentialsQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedCredentialsQuery.qll index ed9c8709fb4d..a14a4ad5e224 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedCredentialsQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedCredentialsQuery.qll @@ -19,54 +19,54 @@ module HardcodedCredentialsConfig implements DataFlow::ConfigSig { predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } - predicate isAdditionalFlowStep(DataFlow::Node src, DataFlow::Node trg) { - exists(Base64::Encode encode | src = encode.getInput() and trg = encode.getOutput()) + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(Base64::Encode encode | node1 = encode.getInput() and node2 = encode.getOutput()) or - trg.(StringOps::ConcatenationRoot).getALeaf() = src and - not exists(src.(StringOps::ConcatenationLeaf).getStringValue()) // to avoid e.g. the ":" in `user + ":" + pass` being flagged as a constant credential. + node2.(StringOps::ConcatenationRoot).getALeaf() = node1 and + not exists(node1.(StringOps::ConcatenationLeaf).getStringValue()) // to avoid e.g. the ":" in `user + ":" + pass` being flagged as a constant credential. or exists(DataFlow::MethodCallNode bufferFrom | bufferFrom = DataFlow::globalVarRef("Buffer").getAMethodCall("from") and - trg = bufferFrom and - src = bufferFrom.getArgument(0) + node2 = bufferFrom and + node1 = bufferFrom.getArgument(0) ) or exists(API::Node n | n = API::moduleImport("jose").getMember(["importSPKI", "importPKCS8", "importX509"]) | - src = n.getACall().getArgument(0) and - trg = n.getReturn().getPromised().asSource() + node1 = n.getACall().getArgument(0) and + node2 = n.getReturn().getPromised().asSource() ) or exists(API::Node n | n = API::moduleImport("jose").getMember(["importSPKI", "importPKCS8", "importX509"]) | - src = n.getACall().getArgument(0) and - trg = n.getReturn().getPromised().asSource() + node1 = n.getACall().getArgument(0) and + node2 = n.getReturn().getPromised().asSource() ) or exists(API::Node n | n = API::moduleImport("jose").getMember("importJWK") | - src = n.getParameter(0).getMember(["x", "y", "n"]).asSink() and - trg = n.getReturn().getPromised().asSource() + node1 = n.getParameter(0).getMember(["x", "y", "n"]).asSink() and + node2 = n.getReturn().getPromised().asSource() ) or exists(DataFlow::CallNode n | n = DataFlow::globalVarRef("TextEncoder").getAnInstantiation().getAMemberCall("encode") | - src = n.getArgument(0) and - trg = n + node1 = n.getArgument(0) and + node2 = n ) or exists(DataFlow::CallNode n | n = DataFlow::globalVarRef("Buffer").getAMemberCall("from") | - src = n.getArgument(0) and - trg = [n, n.getAChainedMethodCall(["toString", "toJSON"])] + node1 = n.getArgument(0) and + node2 = [n, n.getAChainedMethodCall(["toString", "toJSON"])] ) or exists(API::Node n | n = API::moduleImport("jose").getMember("base64url").getMember(["decode", "encode"]) | - src = n.getACall().getArgument(0) and - trg = n.getACall() + node1 = n.getACall().getArgument(0) and + node2 = n.getACall() ) } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeQuery.qll index 24c832eb3c32..550797e1757e 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeQuery.qll @@ -19,9 +19,11 @@ private import HardcodedDataInterpretedAsCodeCustomizations::HardcodedDataInterp module HardcodedDataInterpretedAsCodeConfig implements DataFlow::StateConfigSig { class FlowState = HardcodedDataInterpretedAsCode::FlowState; - predicate isSource(DataFlow::Node source, FlowState lbl) { source.(Source).getAFlowState() = lbl } + predicate isSource(DataFlow::Node source, FlowState state) { + source.(Source).getAFlowState() = state + } - predicate isSink(DataFlow::Node nd, FlowState lbl) { nd.(Sink).getAFlowState() = lbl } + predicate isSink(DataFlow::Node nd, FlowState state) { nd.(Sink).getAFlowState() = state } predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/IncompleteHtmlAttributeSanitizationQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/IncompleteHtmlAttributeSanitizationQuery.qll index 6731754206b1..c04015921257 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/IncompleteHtmlAttributeSanitizationQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/IncompleteHtmlAttributeSanitizationQuery.qll @@ -29,16 +29,16 @@ deprecated private module Label { module IncompleteHtmlAttributeSanitizationConfig implements DataFlow::StateConfigSig { class FlowState = IncompleteHtmlAttributeSanitization::FlowState; - predicate isSource(DataFlow::Node source, FlowState label) { - label = FlowState::character(source.(Source).getAnUnsanitizedCharacter()) + predicate isSource(DataFlow::Node source, FlowState state) { + state = FlowState::character(source.(Source).getAnUnsanitizedCharacter()) } - predicate isSink(DataFlow::Node sink, FlowState label) { - label = FlowState::character(sink.(Sink).getADangerousCharacter()) + predicate isSink(DataFlow::Node sink, FlowState state) { + state = FlowState::character(sink.(Sink).getADangerousCharacter()) } - predicate isBarrier(DataFlow::Node node, FlowState lbl) { - lbl = FlowState::character(node.(StringReplaceCall).getAReplacedString()) + predicate isBarrier(DataFlow::Node node, FlowState state) { + state = FlowState::character(node.(StringReplaceCall).getAReplacedString()) } predicate isBarrier(DataFlow::Node n) { n instanceof Sanitizer } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureRandomnessQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureRandomnessQuery.qll index bf095a637684..93b8b448d92d 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureRandomnessQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureRandomnessQuery.qll @@ -32,13 +32,13 @@ module InsecureRandomnessConfig implements DataFlow::ConfigSig { isSink(node) } - predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { - InsecureRandomness::isAdditionalTaintStep(pred, succ) + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + InsecureRandomness::isAdditionalTaintStep(node1, node2) or // We want to make use of default taint steps but not the default taint sanitizers, as they // generally assume numbers aren't taintable. So we use a data-flow configuration that includes all // taint steps as additional flow steps. - TaintTracking::defaultTaintStep(pred, succ) + TaintTracking::defaultTaintStep(node1, node2) } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/LoopBoundInjectionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/LoopBoundInjectionQuery.qll index b4b799b06da9..2b8a64dbced0 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/LoopBoundInjectionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/LoopBoundInjectionQuery.qll @@ -34,9 +34,9 @@ module LoopBoundInjectionConfig implements DataFlow::StateConfigSig { } predicate isAdditionalFlowStep( - DataFlow::Node src, FlowState inlbl, DataFlow::Node trg, FlowState outlbl + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - TaintedObject::isAdditionalFlowStep(src, inlbl, trg, outlbl) + TaintedObject::isAdditionalFlowStep(node1, state1, node2, state2) } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentQuery.qll index 34a7c3a83462..342c7e9cd931 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentQuery.qll @@ -23,11 +23,11 @@ deprecated private class ConcreteObjectPrototype extends ObjectPrototype { module PrototypePollutingAssignmentConfig implements DataFlow::StateConfigSig { class FlowState = PrototypePollutingAssignment::FlowState; - predicate isSource(DataFlow::Node node, FlowState label) { - node instanceof Source and label = FlowState::taint() + predicate isSource(DataFlow::Node node, FlowState state) { + node instanceof Source and state = FlowState::taint() } - predicate isSink(DataFlow::Node node, FlowState lbl) { node.(Sink).getAFlowState() = lbl } + predicate isSink(DataFlow::Node node, FlowState state) { node.(Sink).getAFlowState() = state } predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer @@ -58,28 +58,28 @@ module PrototypePollutingAssignmentConfig implements DataFlow::StateConfigSig { node = DataFlow::MakeBarrierGuard::getABarrierNode() } - predicate isBarrierOut(DataFlow::Node node, FlowState lbl) { + predicate isBarrierOut(DataFlow::Node node, FlowState state) { // Suppress the value-preserving step src -> dst in `extend(dst, src)`. This is modeled as a value-preserving // step because it preserves all properties, but the destination is not actually Object.prototype. node = any(ExtendCall call).getASourceOperand() and - lbl = FlowState::objectPrototype() + state = FlowState::objectPrototype() } - predicate isBarrierIn(DataFlow::Node node, FlowState lbl) { + predicate isBarrierIn(DataFlow::Node node, FlowState state) { // FIXME: This should only be an in-barrier for the corresponding flow state, but flow-state specific in-barriers are not supported right now. - isSource(node, lbl) + isSource(node, state) } predicate isAdditionalFlowStep( - DataFlow::Node pred, FlowState inlbl, DataFlow::Node succ, FlowState outlbl + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { // Step from x -> obj[x] while switching to the ObjectPrototype label // (If `x` can have the value `__proto__` then the result can be Object.prototype) exists(DynamicPropRead read | - pred = read.getPropertyNameNode() and - succ = read and - inlbl = FlowState::taint() and - outlbl = FlowState::objectPrototype() and + node1 = read.getPropertyNameNode() and + node2 = read and + state1 = FlowState::taint() and + state2 = FlowState::objectPrototype() and // Exclude cases where the property name came from a property enumeration. // If the property name is an own property of the base object, the read won't // return Object.prototype. @@ -93,10 +93,10 @@ module PrototypePollutingAssignmentConfig implements DataFlow::StateConfigSig { // Same as above, but for property projection. exists(PropertyProjection proj | proj.isSingletonProjection() and - pred = proj.getASelector() and - succ = proj and - inlbl = FlowState::taint() and - outlbl = FlowState::objectPrototype() + node1 = proj.getASelector() and + node2 = proj and + state1 = FlowState::taint() and + state2 = FlowState::objectPrototype() ) or // TODO: local field step becomes a jump step, resulting in FPs (closure-lib) @@ -104,22 +104,22 @@ module PrototypePollutingAssignmentConfig implements DataFlow::StateConfigSig { // DataFlow::localFieldStep(pred, succ) none() or - inlbl = FlowState::taint() and - TaintTracking::defaultTaintStep(pred, succ) and - inlbl = outlbl + state1 = FlowState::taint() and + TaintTracking::defaultTaintStep(node1, node2) and + state1 = state2 } DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext } - predicate isBarrier(DataFlow::Node node, FlowState lbl) { - lbl = FlowState::taint() and + predicate isBarrier(DataFlow::Node node, FlowState state) { + state = FlowState::taint() and TaintTracking::defaultSanitizer(node) or // Don't propagate into the receiver, as the method lookups will generally fail on Object.prototype. node instanceof DataFlow::ThisNode and - lbl = FlowState::objectPrototype() + state = FlowState::objectPrototype() or - node = DataFlow::MakeStateBarrierGuard::getABarrierNode(lbl) + node = DataFlow::MakeStateBarrierGuard::getABarrierNode(state) } } @@ -264,10 +264,10 @@ private class PropertyPresenceCheck extends BarrierGuard, DataFlow::ValueNode { not isPropertyPresentOnObjectPrototype(astNode.getPropertyName()) } - override predicate blocksExpr(boolean outcome, Expr e, FlowState label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { e = astNode.getBase() and outcome = true and - label = FlowState::objectPrototype() + state = FlowState::objectPrototype() } } @@ -279,10 +279,10 @@ private class InExprCheck extends BarrierGuard, DataFlow::ValueNode { not isPropertyPresentOnObjectPrototype(astNode.getLeftOperand().getStringValue()) } - override predicate blocksExpr(boolean outcome, Expr e, FlowState label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { e = astNode.getRightOperand() and outcome = true and - label = FlowState::objectPrototype() + state = FlowState::objectPrototype() } } @@ -290,10 +290,10 @@ private class InExprCheck extends BarrierGuard, DataFlow::ValueNode { private class InstanceofCheck extends BarrierGuard, DataFlow::ValueNode { override InstanceofExpr astNode; - override predicate blocksExpr(boolean outcome, Expr e, FlowState label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { e = astNode.getLeftOperand() and outcome = true and - label = FlowState::objectPrototype() + state = FlowState::objectPrototype() } } @@ -311,10 +311,10 @@ private class TypeofCheck extends BarrierGuard, DataFlow::ValueNode { ) } - override predicate blocksExpr(boolean outcome, Expr e, FlowState label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { polarity = outcome and e = operand and - label = FlowState::objectPrototype() + state = FlowState::objectPrototype() } } @@ -332,10 +332,10 @@ class NumberGuard extends BarrierGuard instanceof DataFlow::CallNode { private class IsArrayCheck extends BarrierGuard, DataFlow::CallNode { IsArrayCheck() { this = DataFlow::globalVarRef("Array").getAMemberCall("isArray") } - override predicate blocksExpr(boolean outcome, Expr e, FlowState label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { e = this.getArgument(0).asExpr() and outcome = true and - label = FlowState::objectPrototype() + state = FlowState::objectPrototype() } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionQuery.qll index f5d54396ca7c..03d5e0c62a1b 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutionQuery.qll @@ -32,9 +32,9 @@ module PrototypePollutionConfig implements DataFlow::StateConfigSig { predicate isSink(DataFlow::Node node, FlowState state) { node.(Sink).getAFlowState() = state } predicate isAdditionalFlowStep( - DataFlow::Node src, FlowState inlbl, DataFlow::Node dst, FlowState outlbl + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - TaintedObject::isAdditionalFlowStep(src, inlbl, dst, outlbl) + TaintedObject::isAdditionalFlowStep(node1, state1, node2, state2) } predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet contents) { diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/RequestForgeryQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/RequestForgeryQuery.qll index 09c956d12ee9..74317ebcc083 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/RequestForgeryQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/RequestForgeryQuery.qll @@ -23,8 +23,8 @@ module RequestForgeryConfig implements DataFlow::ConfigSig { predicate isBarrierOut(DataFlow::Node node) { sanitizingPrefixEdge(node, _) } - predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { - isAdditionalRequestForgeryStep(pred, succ) + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + isAdditionalRequestForgeryStep(node1, node2) } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ResourceExhaustionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ResourceExhaustionQuery.qll index 65481b929d01..95360d0face4 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ResourceExhaustionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ResourceExhaustionQuery.qll @@ -24,8 +24,8 @@ module ResourceExhaustionConfig implements DataFlow::ConfigSig { node = DataFlow::MakeBarrierGuard::getABarrierNode() } - predicate isAdditionalFlowStep(DataFlow::Node src, DataFlow::Node dst) { - isNumericFlowStep(src, dst) + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + isNumericFlowStep(node1, node2) } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/SecondOrderCommandInjectionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/SecondOrderCommandInjectionQuery.qll index 2de01d29b61f..16d15b42ce47 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/SecondOrderCommandInjectionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/SecondOrderCommandInjectionQuery.qll @@ -37,15 +37,15 @@ module SecondOrderCommandInjectionConfig implements DataFlow::StateConfigSig { } predicate isAdditionalFlowStep( - DataFlow::Node src, FlowState inlbl, DataFlow::Node trg, FlowState outlbl + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - TaintedObject::isAdditionalFlowStep(src, inlbl, trg, outlbl) + TaintedObject::isAdditionalFlowStep(node1, state1, node2, state2) or // We're not using a taint-tracking config because taint steps would then apply to all flow states. // So we use a plain data flow config and manually add the default taint steps. - inlbl.isTaint() and - TaintTracking::defaultTaintStep(src, trg) and - inlbl = outlbl + state1.isTaint() and + TaintTracking::defaultTaintStep(node1, node2) and + state1 = state2 } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ServerSideUrlRedirectQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ServerSideUrlRedirectQuery.qll index e92aa0c9607d..dc45a6c5614e 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ServerSideUrlRedirectQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ServerSideUrlRedirectQuery.qll @@ -24,10 +24,10 @@ module ServerSideUrlRedirectConfig implements DataFlow::ConfigSig { predicate isBarrierOut(DataFlow::Node node) { hostnameSanitizingPrefixEdge(node, _) } - predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { exists(HtmlSanitizerCall call | - pred = call.getInput() and - succ = call + node1 = call.getInput() and + node2 = call ) } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/SqlInjectionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/SqlInjectionQuery.qll index 3a5f0e41bfaf..f91a9ce27d3c 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/SqlInjectionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/SqlInjectionQuery.qll @@ -20,15 +20,15 @@ module SqlInjectionConfig implements DataFlow::ConfigSig { predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } - predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { exists(LdapJS::TaintPreservingLdapFilterStep filter | - pred = filter.getInput() and - succ = filter.getOutput() + node1 = filter.getInput() and + node2 = filter.getOutput() ) or exists(HtmlSanitizerCall call | - pred = call.getInput() and - succ = call + node1 = call.getInput() and + node2 = call ) } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/TemplateObjectInjectionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/TemplateObjectInjectionQuery.qll index f71ddfdb6fa1..66e401d40ac1 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/TemplateObjectInjectionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/TemplateObjectInjectionQuery.qll @@ -35,15 +35,15 @@ module TemplateObjectInjectionConfig implements DataFlow::StateConfigSig { } predicate isAdditionalFlowStep( - DataFlow::Node src, FlowState inlbl, DataFlow::Node trg, FlowState outlbl + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - TaintedObject::isAdditionalFlowStep(src, inlbl, trg, outlbl) + TaintedObject::isAdditionalFlowStep(node1, state1, node2, state2) or // We're not using a taint-tracking config because taint steps would then apply to all flow states. // So we use a plain data flow config and manually add the default taint steps. - inlbl.isTaint() and - TaintTracking::defaultTaintStep(src, trg) and - inlbl = outlbl + state1.isTaint() and + TaintTracking::defaultTaintStep(node1, node2) and + state1 = state2 } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeCodeConstruction.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeCodeConstruction.qll index 5e2c3d8f195b..fc1e6c79b384 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeCodeConstruction.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeCodeConstruction.qll @@ -26,9 +26,9 @@ module UnsafeCodeConstruction { predicate isBarrier(DataFlow::Node node) { node instanceof CodeInjection::Sanitizer } - predicate isAdditionalFlowStep(DataFlow::Node src, DataFlow::Node trg) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { // HTML sanitizers are insufficient protection against code injection - src = trg.(HtmlSanitizerCall).getInput() + node1 = node2.(HtmlSanitizerCall).getInput() or none() // TODO: localFieldStep is too expensive with dataflow2 diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessQuery.qll index 57ea77d1d4a3..e88c4b7d83b1 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessQuery.qll @@ -24,11 +24,11 @@ deprecated private class ConcreteUnsafeFunction extends UnsafeFunction { module UnsafeDynamicMethodAccessConfig implements DataFlow::StateConfigSig { class FlowState = UnsafeDynamicMethodAccess::FlowState; - predicate isSource(DataFlow::Node source, FlowState label) { - source.(Source).getAFlowState() = label + predicate isSource(DataFlow::Node source, FlowState state) { + source.(Source).getAFlowState() = state } - predicate isSink(DataFlow::Node sink, FlowState label) { sink.(Sink).getAFlowState() = label } + predicate isSink(DataFlow::Node sink, FlowState state) { sink.(Sink).getAFlowState() = state } predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer @@ -37,44 +37,44 @@ module UnsafeDynamicMethodAccessConfig implements DataFlow::StateConfigSig { not StringConcatenation::isCoercion(node) } - predicate isBarrier(DataFlow::Node node, FlowState label) { + predicate isBarrier(DataFlow::Node node, FlowState state) { TaintTracking::defaultSanitizer(node) and - label = FlowState::taint() + state = FlowState::taint() } /** An additional flow step for use in both this configuration and the legacy configuration. */ additional predicate additionalFlowStep( - DataFlow::Node src, FlowState srclabel, DataFlow::Node dst, FlowState dstlabel + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { // Reading a property of the global object or of a function exists(DataFlow::PropRead read | PropertyInjection::hasUnsafeMethods(read.getBase().getALocalSource()) and - src = read.getPropertyNameExpr().flow() and - dst = read and - srclabel = FlowState::taint() and - dstlabel = FlowState::unsafeFunction() + node1 = read.getPropertyNameExpr().flow() and + node2 = read and + state1 = FlowState::taint() and + state2 = FlowState::unsafeFunction() ) or // Reading a chain of properties from any object with a prototype can lead to Function exists(PropertyProjection proj | not PropertyInjection::isPrototypeLessObject(proj.getObject().getALocalSource()) and - src = proj.getASelector() and - dst = proj and - srclabel = FlowState::taint() and - dstlabel = FlowState::unsafeFunction() + node1 = proj.getASelector() and + node2 = proj and + state1 = FlowState::taint() and + state2 = FlowState::unsafeFunction() ) } predicate isAdditionalFlowStep( - DataFlow::Node src, FlowState srclabel, DataFlow::Node dst, FlowState dstlabel + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - additionalFlowStep(src, srclabel, dst, dstlabel) + additionalFlowStep(node1, state1, node2, state2) or // We're not using a taint-tracking config because taint steps would then apply to all flow states. // So we use a plain data flow config and manually add the default taint steps. - srclabel = FlowState::taint() and - TaintTracking::defaultTaintStep(src, dst) and - srclabel = dstlabel + state1 = FlowState::taint() and + TaintTracking::defaultTaintStep(node1, node2) and + state1 = state2 } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionQuery.qll index 4ca7b4787a43..5fdf3825405f 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionQuery.qll @@ -45,7 +45,7 @@ module UnsafeHtmlConstructionConfig implements DataFlow::StateConfigSig { } predicate isAdditionalFlowStep( - DataFlow::Node pred, FlowState inlbl, DataFlow::Node succ, FlowState outlbl + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { // TODO: localFieldStep is too expensive with dataflow2 // DataFlow::localFieldStep(pred, succ) and @@ -53,16 +53,16 @@ module UnsafeHtmlConstructionConfig implements DataFlow::StateConfigSig { // outlbl.isTaint() none() or - TaintedObject::isAdditionalFlowStep(pred, inlbl, succ, outlbl) + TaintedObject::isAdditionalFlowStep(node1, state1, node2, state2) or // property read from a tainted object is considered tainted - succ.(DataFlow::PropRead).getBase() = pred and - inlbl.isTaintedObject() and - outlbl.isTaint() + node2.(DataFlow::PropRead).getBase() = node1 and + state1.isTaintedObject() and + state2.isTaint() or - TaintTracking::defaultTaintStep(pred, succ) and - inlbl.isTaint() and - outlbl = inlbl + TaintTracking::defaultTaintStep(node1, node2) and + state1.isTaint() and + state2 = state1 } DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeJQueryPluginQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeJQueryPluginQuery.qll index 1860ffa3be6f..e7bf16cf0c32 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeJQueryPluginQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeJQueryPluginQuery.qll @@ -22,13 +22,13 @@ module UnsafeJQueryPluginConfig implements DataFlow::ConfigSig { node = DataFlow::MakeBarrierGuard::getABarrierNode() } - predicate isAdditionalFlowStep(DataFlow::Node src, DataFlow::Node sink) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node sink) { // jQuery plugins tend to be implemented as classes that store data in fields initialized by the constructor. // TODO: localFieldStep is too expensive with dataflow2 // DataFlow::localFieldStep(pred, succ) none() or - aliasPropertyPresenceStep(src, sink) + aliasPropertyPresenceStep(node1, sink) } predicate isBarrierOut(DataFlow::Node node) { diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionQuery.qll index 1704bf3e3e6f..5eb11826bffd 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionQuery.qll @@ -26,7 +26,7 @@ module UnsafeShellCommandConstructionConfig implements DataFlow::ConfigSig { node = TaintTracking::AdHocWhitelistCheckSanitizer::getABarrierNode() } - predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { none() // TODO: localFieldStep is too expensive with dataflow2 // DataFlow::localFieldStep(pred, succ) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallQuery.qll index fdc8155deb34..d8b29fca9014 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallQuery.qll @@ -30,19 +30,19 @@ deprecated private class ConcreteMaybeFromProto extends MaybeFromProto { module UnvalidatedDynamicMethodCallConfig implements DataFlow::StateConfigSig { class FlowState = UnvalidatedDynamicMethodCall::FlowState; - predicate isSource(DataFlow::Node source, FlowState label) { - source.(Source).getAFlowState() = label + predicate isSource(DataFlow::Node source, FlowState state) { + source.(Source).getAFlowState() = state } - predicate isSink(DataFlow::Node sink, FlowState label) { sink.(Sink).getAFlowState() = label } + predicate isSink(DataFlow::Node sink, FlowState state) { sink.(Sink).getAFlowState() = state } - predicate isBarrier(DataFlow::Node node, FlowState label) { - node.(Sanitizer).getAFlowState() = label + predicate isBarrier(DataFlow::Node node, FlowState state) { + node.(Sanitizer).getAFlowState() = state or TaintTracking::defaultSanitizer(node) and - label = FlowState::taint() + state = FlowState::taint() or - node = DataFlow::MakeStateBarrierGuard::getABarrierNode(label) + node = DataFlow::MakeStateBarrierGuard::getABarrierNode(state) } predicate isBarrier(DataFlow::Node node) { @@ -50,33 +50,33 @@ module UnvalidatedDynamicMethodCallConfig implements DataFlow::StateConfigSig { } predicate isAdditionalFlowStep( - DataFlow::Node src, FlowState srclabel, DataFlow::Node dst, FlowState dstlabel + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { exists(DataFlow::PropRead read | - src = read.getPropertyNameExpr().flow() and - dst = read and - srclabel = FlowState::taint() and + node1 = read.getPropertyNameExpr().flow() and + node2 = read and + state1 = FlowState::taint() and ( - dstlabel = FlowState::maybeNonFunction() + state2 = FlowState::maybeNonFunction() or // a property of `Object.create(null)` cannot come from a prototype not PropertyInjection::isPrototypeLessObject(read.getBase().getALocalSource()) and - dstlabel = FlowState::maybeFromProto() + state2 = FlowState::maybeFromProto() ) and // avoid overlapping results with unsafe dynamic method access query not PropertyInjection::hasUnsafeMethods(read.getBase().getALocalSource()) ) or exists(DataFlow::SourceNode base, DataFlow::CallNode get | get = base.getAMethodCall("get") | - src = get.getArgument(0) and - dst = get + node1 = get.getArgument(0) and + node2 = get ) and - srclabel = FlowState::taint() and - dstlabel = FlowState::maybeNonFunction() + state1 = FlowState::taint() and + state2 = FlowState::maybeNonFunction() or - srclabel = FlowState::taint() and - TaintTracking::defaultTaintStep(src, dst) and - srclabel = dstlabel + state1 = FlowState::taint() and + TaintTracking::defaultTaintStep(node1, node2) and + state1 = state2 } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomQuery.qll index 7d6564fce10c..2313751d9e70 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomQuery.qll @@ -26,9 +26,9 @@ module XssThroughDomConfig implements DataFlow::ConfigSig { node = Shared::BarrierGuard::getABarrierNode() } - predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { - succ = DataFlow::globalVarRef("URL").getAMemberCall("createObjectURL") and - pred = succ.(DataFlow::InvokeNode).getArgument(0) + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + node2 = DataFlow::globalVarRef("URL").getAMemberCall("createObjectURL") and + node1 = node2.(DataFlow::InvokeNode).getArgument(0) } } diff --git a/javascript/ql/lib/semmle/javascript/security/regexp/PolynomialReDoSQuery.qll b/javascript/ql/lib/semmle/javascript/security/regexp/PolynomialReDoSQuery.qll index 3046febcc2ab..0c0f502bb06b 100644 --- a/javascript/ql/lib/semmle/javascript/security/regexp/PolynomialReDoSQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/regexp/PolynomialReDoSQuery.qll @@ -22,7 +22,7 @@ module PolynomialReDoSConfig implements DataFlow::ConfigSig { DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext } - predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { none() // TODO: localFieldStep is too expensive with dataflow2 // DataFlow::localFieldStep(pred, succ) diff --git a/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.ql b/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.ql index 22aa99e7e55c..f0734b877c96 100644 --- a/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.ql +++ b/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.ql @@ -65,27 +65,27 @@ module CodeInjectionConfig implements DataFlow::StateConfigSig { } } - predicate isSource(DataFlow::Node source, FlowState label) { - source instanceof ActiveThreatModelSource and label = TTaint() + predicate isSource(DataFlow::Node source, FlowState state) { + source instanceof ActiveThreatModelSource and state = TTaint() } predicate isSink(DataFlow::Node sink) { sink instanceof DynamicImport } - predicate isSink(DataFlow::Node sink, FlowState label) { - sink instanceof WorkerThreads and label = TUrlConstructor() + predicate isSink(DataFlow::Node sink, FlowState state) { + sink instanceof WorkerThreads and state = TUrlConstructor() } predicate isBarrier(DataFlow::Node node) { node instanceof Barrier } predicate isAdditionalFlowStep( - DataFlow::Node pred, FlowState predlbl, DataFlow::Node succ, FlowState succlbl + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - exists(DataFlow::NewNode newUrl | succ = newUrl | + exists(DataFlow::NewNode newUrl | node2 = newUrl | newUrl = DataFlow::globalVarRef("URL").getAnInstantiation() and - pred = newUrl.getArgument(0) + node1 = newUrl.getArgument(0) ) and - predlbl = TTaint() and - succlbl = TUrlConstructor() + state1 = TTaint() and + state2 = TUrlConstructor() } } diff --git a/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.ql b/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.ql index 0f638167dc64..e66406f84053 100644 --- a/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.ql +++ b/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.ql @@ -21,15 +21,15 @@ module EnvValueAndKeyInjectionConfig implements DataFlow::ConfigSig { sink = valueOfEnv() } - predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { exists(DataFlow::InvokeNode ikn | ikn = DataFlow::globalVarRef("Object").getAMemberInvocation("keys") | - pred = ikn.getArgument(0) and + node1 = ikn.getArgument(0) and ( - succ = ikn.getAChainedMethodCall(["filter", "map"]) or - succ = ikn or - succ = ikn.getAChainedMethodCall("forEach").getABoundCallbackParameter(0, 0) + node2 = ikn.getAChainedMethodCall(["filter", "map"]) or + node2 = ikn or + node2 = ikn.getAChainedMethodCall("forEach").getABoundCallbackParameter(0, 0) ) ) } diff --git a/javascript/ql/src/experimental/Security/CWE-522-DecompressionBombs/DecompressionBombs.ql b/javascript/ql/src/experimental/Security/CWE-522-DecompressionBombs/DecompressionBombs.ql index 29e62b85d4e8..17e3f1f2fd9e 100644 --- a/javascript/ql/src/experimental/Security/CWE-522-DecompressionBombs/DecompressionBombs.ql +++ b/javascript/ql/src/experimental/Security/CWE-522-DecompressionBombs/DecompressionBombs.ql @@ -19,9 +19,9 @@ module DecompressionBombConfig implements DataFlow::ConfigSig { predicate isSink(DataFlow::Node sink) { sink instanceof DecompressionBomb::Sink } - predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { exists(DecompressionBomb::AdditionalTaintStep addstep | - addstep.isAdditionalTaintStep(pred, succ) + addstep.isAdditionalTaintStep(node1, node2) ) } } From cf6d166d29ca20e2abfd516b390b0ae579f0d9db Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 15:00:50 +0100 Subject: [PATCH 33/37] JS: Also update tutorial code --- ...lyzing-data-flow-in-javascript-and-typescript.rst | 12 ++++++------ .../Global data flow/query4.ql | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/codeql/codeql-language-guides/analyzing-data-flow-in-javascript-and-typescript.rst b/docs/codeql/codeql-language-guides/analyzing-data-flow-in-javascript-and-typescript.rst index 44da0912d4a2..2a65fbe5dc59 100644 --- a/docs/codeql/codeql-language-guides/analyzing-data-flow-in-javascript-and-typescript.rst +++ b/docs/codeql/codeql-language-guides/analyzing-data-flow-in-javascript-and-typescript.rst @@ -416,11 +416,11 @@ additional taint step from the first argument of ``resolveSymlinks`` to its resu // ... - predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { exists(DataFlow::CallNode c | c = DataFlow::moduleImport("resolve-symlinks").getACall() and - pred = c.getArgument(0) and - succ = c + node1 = c.getArgument(0) and + node2 = c ) } } @@ -431,11 +431,11 @@ to wrap it in a new subclass of ``TaintTracking::SharedTaintStep`` like this: .. code-block:: ql class StepThroughResolveSymlinks extends TaintTracking::SharedTaintStep { - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { + override predicate step(DataFlow::Node node1, DataFlow::Node node2) { exists(DataFlow::CallNode c | c = DataFlow::moduleImport("resolve-symlinks").getACall() and - pred = c.getArgument(0) and - succ = c + node1 = c.getArgument(0) and + node2 = c ) } } diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query4.ql b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query4.ql index dff069d72e22..b4980bc91730 100644 --- a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query4.ql +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query4.ql @@ -9,11 +9,11 @@ module CommandLineFileNameConfig implements DataFlow::ConfigSig { DataFlow::moduleMember("fs", "readFile").getACall().getArgument(0) = sink } - predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { exists(DataFlow::CallNode c | c = DataFlow::moduleImport("resolve-symlinks").getACall() and - pred = c.getArgument(0) and - succ = c + node1 = c.getArgument(0) and + node2 = c ) } } From db00dad0332626fd4a4e84fff349f139c23b39e3 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 15:45:18 +0100 Subject: [PATCH 34/37] JS: Avoid deprecation warnings in some tests --- .../ql/test/library-tests/FlowLabels/DefaultFlowLabels.ql | 3 +++ .../LabelledBarrierGuards/LabelledBarrierGuards.ql | 3 +++ 2 files changed, 6 insertions(+) diff --git a/javascript/ql/test/library-tests/FlowLabels/DefaultFlowLabels.ql b/javascript/ql/test/library-tests/FlowLabels/DefaultFlowLabels.ql index 026933fc123f..c1a8739afdd8 100644 --- a/javascript/ql/test/library-tests/FlowLabels/DefaultFlowLabels.ql +++ b/javascript/ql/test/library-tests/FlowLabels/DefaultFlowLabels.ql @@ -1,3 +1,6 @@ +// Delete test when FlowLabel has been removed +deprecated module; + import javascript // Check which flow labels are materialized by importing `javascript.qll`. diff --git a/javascript/ql/test/library-tests/LabelledBarrierGuards/LabelledBarrierGuards.ql b/javascript/ql/test/library-tests/LabelledBarrierGuards/LabelledBarrierGuards.ql index ad8b63ea13e9..b6dc9bb59681 100644 --- a/javascript/ql/test/library-tests/LabelledBarrierGuards/LabelledBarrierGuards.ql +++ b/javascript/ql/test/library-tests/LabelledBarrierGuards/LabelledBarrierGuards.ql @@ -1,3 +1,6 @@ +// Delete test when LabelledBarrierGuards have been removed +deprecated module; + import javascript class CustomFlowLabel extends DataFlow::FlowLabel { From 0b2914ff13b3dd403dd1653df8a39ea4ee4434d0 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 16:21:55 +0100 Subject: [PATCH 35/37] JS: A few more deprecation updates --- .../security/dataflow/BuildArtifactLeakCustomizations.qll | 4 +++- .../security/dataflow/InsecureDownloadCustomizations.qll | 2 +- .../security/dataflow/PostMessageStarCustomizations.qll | 8 ++++++-- .../security/dataflow/UnsafeDynamicMethodAccessQuery.qll | 1 - .../dataflow/UnsafeJQueryPluginCustomizations.qll | 2 -- 5 files changed, 10 insertions(+), 7 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/BuildArtifactLeakCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/BuildArtifactLeakCustomizations.qll index b3033eb4cd20..70641af0286a 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/BuildArtifactLeakCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/BuildArtifactLeakCustomizations.qll @@ -14,9 +14,11 @@ module BuildArtifactLeak { */ abstract class Sink extends DataFlow::Node { /** + * DEPRECATED. This query no longer uses flow state. + * * Gets a data-flow label that leaks information for this sink. */ - DataFlow::FlowLabel getLabel() { result.isTaint() } + deprecated DataFlow::FlowLabel getLabel() { result.isTaint() } } /** diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadCustomizations.qll index d4488840bb4f..c7acce602b72 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadCustomizations.qll @@ -88,7 +88,7 @@ module InsecureDownload { /** * Flow-labels for reasoning about download of sensitive file through insecure connection. */ - module Label { + deprecated module Label { /** * A flow-label for file URLs that are both sensitive and downloaded over an insecure connection. */ diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/PostMessageStarCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/PostMessageStarCustomizations.qll index 736e47daaf67..62f87e29b0b9 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/PostMessageStarCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/PostMessageStarCustomizations.qll @@ -24,16 +24,20 @@ module PostMessageStar { abstract class Sanitizer extends DataFlow::Node { } /** + * DEPRECATED. This query no longer uses flow state. + * * A flow label representing an object with at least one tainted property. */ - abstract class PartiallyTaintedObject extends DataFlow::FlowLabel { + abstract deprecated class PartiallyTaintedObject extends DataFlow::FlowLabel { PartiallyTaintedObject() { this = "partially tainted object" } } /** + * DEPRECATED. This query no longer uses flow state. + * * Gets either a standard flow label or the partial-taint label. */ - DataFlow::FlowLabel anyLabel() { + deprecated DataFlow::FlowLabel anyLabel() { result.isDataOrTaint() or result instanceof PartiallyTaintedObject } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessQuery.qll index e88c4b7d83b1..86a225f894a8 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessQuery.qll @@ -9,7 +9,6 @@ import javascript import PropertyInjectionShared -private import DataFlow::FlowLabel import UnsafeDynamicMethodAccessCustomizations::UnsafeDynamicMethodAccess private import UnsafeDynamicMethodAccessCustomizations::UnsafeDynamicMethodAccess as UnsafeDynamicMethodAccess diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeJQueryPluginCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeJQueryPluginCustomizations.qll index d9c1daec4e46..3fb2827707e3 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeJQueryPluginCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeJQueryPluginCustomizations.qll @@ -9,8 +9,6 @@ private import semmle.javascript.dataflow.InferredTypes import semmle.javascript.security.dataflow.DomBasedXssCustomizations module UnsafeJQueryPlugin { - private import DataFlow::FlowLabel - /** * A data flow source for unsafe jQuery plugins. */ From 947b785d47b08d2bf013ea6a972f76f61ac1fca9 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Dec 2024 16:22:14 +0100 Subject: [PATCH 36/37] JS: Remove reference to deprecated step relation that's empty anyway --- .../dataflow/ExternalAPIUsedWithUntrustedDataCustomizations.qll | 1 - javascript/ql/src/meta/analysis-quality/TaintSteps.ql | 2 -- javascript/ql/src/meta/analysis-quality/UnmodelledSteps.ql | 2 -- 3 files changed, 5 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ExternalAPIUsedWithUntrustedDataCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ExternalAPIUsedWithUntrustedDataCustomizations.qll index fac7b95fe803..72fdafaad501 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ExternalAPIUsedWithUntrustedDataCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ExternalAPIUsedWithUntrustedDataCustomizations.qll @@ -252,7 +252,6 @@ module ExternalApiUsedWithUntrustedData { | TaintTracking::sharedTaintStep(arg, _) or DataFlow::SharedFlowStep::step(arg, _) or - DataFlow::SharedFlowStep::step(arg, _, _, _) or DataFlow::SharedFlowStep::loadStep(arg, _, _) or DataFlow::SharedFlowStep::storeStep(arg, _, _) or DataFlow::SharedFlowStep::loadStoreStep(arg, _, _) diff --git a/javascript/ql/src/meta/analysis-quality/TaintSteps.ql b/javascript/ql/src/meta/analysis-quality/TaintSteps.ql index be16675f849f..5c2cfcf1b80b 100644 --- a/javascript/ql/src/meta/analysis-quality/TaintSteps.ql +++ b/javascript/ql/src/meta/analysis-quality/TaintSteps.ql @@ -17,8 +17,6 @@ predicate relevantStep(DataFlow::Node pred, DataFlow::Node succ) { or DataFlow::SharedFlowStep::step(pred, succ) or - DataFlow::SharedFlowStep::step(pred, succ, _, _) - or DataFlow::SharedFlowStep::loadStep(pred, succ, _) or DataFlow::SharedFlowStep::storeStep(pred, succ, _) diff --git a/javascript/ql/src/meta/analysis-quality/UnmodelledSteps.ql b/javascript/ql/src/meta/analysis-quality/UnmodelledSteps.ql index d1a4ae1d0998..429682897059 100644 --- a/javascript/ql/src/meta/analysis-quality/UnmodelledSteps.ql +++ b/javascript/ql/src/meta/analysis-quality/UnmodelledSteps.ql @@ -24,8 +24,6 @@ predicate unmodeled(API::Node callee, API::CallNode call, DataFlow::Node pred, D or DataFlow::SharedFlowStep::step(_, succ) or - DataFlow::SharedFlowStep::step(_, succ, _, _) - or DataFlow::SharedFlowStep::loadStep(_, succ, _) or DataFlow::SharedFlowStep::storeStep(_, succ, _) From e5ae7e023109fbd4ce8cc89762241bcf0287875b Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 16 Dec 2024 10:48:17 +0100 Subject: [PATCH 37/37] JS: Fix bad join in isOptionallySanitizedEdgeInternal This was previously called from isBarrier(node, state) but without restricting the state. The call was therefore moved to isBarrier(node), but this caused some optimisation changes resulting in a bad join. --- .../security/dataflow/DomBasedXssCustomizations.qll | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll index 731ef1eb721c..b9f27c6a8c2e 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll @@ -329,6 +329,12 @@ module DomBasedXss { */ deprecated predicate isOptionallySanitizedEdge = isOptionallySanitizedEdgeInternal/2; + bindingset[call] + pragma[inline_late] + private SsaVariable getSanitizedSsaVariable(HtmlSanitizerCall call) { + call.getAnArgument().asExpr().(VarAccess).getVariable() = result.getSourceVariable() + } + private predicate isOptionallySanitizedEdgeInternal(DataFlow::Node pred, DataFlow::Node succ) { exists(HtmlSanitizerCall sanitizer | // sanitized = sanitize ? sanitizer(source) : source; @@ -348,7 +354,7 @@ module DomBasedXss { count(phi.getAnInput()) = 2 and not a = b and sanitizer = DataFlow::valueNode(a.getDef().getSource()) and - sanitizer.getAnArgument().asExpr().(VarAccess).getVariable() = b.getSourceVariable() + getSanitizedSsaVariable(sanitizer) = b | pred = DataFlow::ssaDefinitionNode(b) and succ = DataFlow::ssaDefinitionNode(phi)