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/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/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. */ 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..52e1e0d00f38 --- /dev/null +++ b/javascript/ql/lib/semmle/javascript/security/CommonFlowState.qll @@ -0,0 +1,90 @@ +/** + * Contains a class with flow states that are used by multiple queries. + */ + +private import javascript +private import TaintedUrlSuffixCustomizations +private import TaintedObjectCustomizations + +private newtype TFlowState = + TTaint() or + TTaintedUrlSuffix() or + TTaintedPrefix() or + TTaintedObject() + +/** + * 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() } + + /** + * Holds if this represents a string whose prefix is known to be tainted. + */ + 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" + or + 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. */ + deprecated DataFlow::FlowLabel toFlowLabel() { + this.isTaint() and result.isTaint() + or + this.isTaintedUrlSuffix() and result = TaintedUrlSuffix::label() + or + this.isTaintedPrefix() and result = "PrefixString" + or + this.isTaintedObject() and result = TaintedObject::label() + } +} + +/** 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() } + + /** + * Gets the flow state representing a string whose prefix is known to be tainted. + */ + 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..a300291ae9cd 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,56 +22,67 @@ module TaintedObject { import TaintedObjectCustomizations::TaintedObject // Materialize flow labels - private class ConcreteTaintedObjectLabel extends TaintedObjectLabel { + deprecated private class ConcreteTaintedObjectLabel extends TaintedObjectLabel { 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)) + } + /** * 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 node1, FlowState state1, Node node2, FlowState state2) { // JSON parsers map tainted inputs to tainted JSON - inlbl.isDataOrTaint() and - outlbl = label() 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 = label() and - outlbl = label() and - trg.(PropRead).getBase() = src + state1.isTaintedObject() and + state2.isTaintedObject() and + node2.(PropRead).getBase() = node1 or // Property projection preserves deep object taint - inlbl = label() and - outlbl = label() and - trg.(PropertyProjection).getObject() = src + state1.isTaintedObject() and + state2.isTaintedObject() and + node2.(PropertyProjection).getObject() = node1 or // Extending objects preserves deep object taint - inlbl = label() and - outlbl = label() 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 = label() and - outlbl = label() and + state1.isTaintedObject() and + state2.isTaintedObject() and exists(ObjectLiteralNode obj | - src = obj.getASpreadProperty() and - trg = obj + node1 = obj.getASpreadProperty() and + node2 = obj ) } /** + * 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 { @@ -85,12 +96,12 @@ 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, FlowLabel 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) { - this.blocksExpr(outcome, e, label) + this.blocksExpr(outcome, e, FlowState::fromFlowLabel(label)) } /** DEPRECATED. Use `blocksExpr` instead. */ @@ -111,7 +122,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 +144,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 +172,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 +186,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. 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 af69ede9197b..ee6cf5da6d94 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,25 +7,26 @@ 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 + import CommonFlowState /** * The flow label representing a URL with a tainted query and fragment part. * * 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() { @@ -37,22 +38,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 `node1 -> node2` 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 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. // @@ -61,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 = label() 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 = label() 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 @@ -100,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" ) @@ -109,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/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/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/ClientSideUrlRedirectCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll index 3601a8114545..1b987ea2679e 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll @@ -8,12 +8,17 @@ import javascript private import semmle.javascript.security.TaintedUrlSuffixCustomizations module ClientSideUrlRedirect { + import semmle.javascript.security.CommonFlowState + /** * 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 +55,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..bc0e1354757e 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; + import semmle.javascript.security.CommonFlowState - 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 state) { isSink(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(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 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/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) } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll index 8dd89667959d..b9f27c6a8c2e 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)) } } @@ -328,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; @@ -347,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) @@ -379,20 +386,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 } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssCustomizations.qll index 20a710a9da62..70281110a5fb 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssCustomizations.qll @@ -14,15 +14,51 @@ 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" + } + + /** Gets the corresponding flow label. */ + deprecated DataFlow::FlowLabel toFlowLabel() { + this = TThrown() and result.isTaint() + or + this = TNotYetThrown() and result instanceof NotYetThrown + } + } + + /** 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. */ + 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. Use `getAFlowState()` instead. */ + deprecated DataFlow::FlowLabel getAFlowLabel() { result = this.getAFlowState().toFlowLabel() } /** * Gets a human-readable description of what type of error this refers to. @@ -33,17 +69,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..71603d38ecd6 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,12 +146,12 @@ module ExceptionXssConfig implements DataFlow::StateConfigSig { } predicate isAdditionalFlowStep( - DataFlow::Node pred, DataFlow::FlowLabel inlbl, DataFlow::Node succ, DataFlow::FlowLabel outlbl + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - inlbl instanceof NotYetThrown and - (outlbl.isTaint() or outlbl instanceof NotYetThrown) and - canThrowSensitiveInformation(pred) and - succ = getExceptionTarget(pred) + state1 = FlowState::notYetThrown() and + state2 = [FlowState::thrown(), FlowState::notYetThrown()] and + canThrowSensitiveInformation(node1) and + node2 = getExceptionTarget(node1) } } @@ -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. 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/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/HardcodedDataInterpretedAsCodeCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeCustomizations.qll index 14d2c8fc1489..9ec6f2d5b4ad 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeCustomizations.qll @@ -8,20 +8,59 @@ 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" + } + + /** Gets the corresponding flow label. */ + deprecated DataFlow::FlowLabel toFlowLabel() { + this = TUnmodified() and result.isData() + or + this = TModified() and result.isTaint() + } + } + + /** 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. */ + 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 +89,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 +100,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..550797e1757e 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/HardcodedDataInterpretedAsCodeQuery.qll @@ -10,29 +10,29 @@ 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 state) { + source.(Source).getAFlowState() = state } - predicate isSink(DataFlow::Node nd, DataFlow::FlowLabel lbl) { nd.(Sink).getLabel() = lbl } + predicate isSink(DataFlow::Node nd, FlowState state) { nd.(Sink).getAFlowState() = state } 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() } } 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..c04015921257 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 state) { + state = 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 state) { + state = 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 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/InsecureDownloadCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadCustomizations.qll index dc383df448c3..c7acce602b72 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/InsecureDownloadCustomizations.qll @@ -10,14 +10,54 @@ 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" + } + + /** Gets the corresponding flow label. */ + 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 { + /** Gets the flow state corresponding to `label`. */ + 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 +70,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() } } /** @@ -43,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. */ @@ -71,11 +116,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 +158,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 +190,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 7f7d3341d5ae..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 } } @@ -29,7 +28,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. @@ -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) { 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/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..2b8a64dbced0 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 node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - TaintedObject::step(src, trg, inlbl, outlbl) + TaintedObject::isAdditionalFlowStep(node1, state1, node2, state2) } } 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)) } } 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/PrototypePollutingAssignmentCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentCustomizations.qll index 5ac278924e24..bb2f9739501c 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/PrototypePollutingAssignmentCustomizations.qll @@ -10,6 +10,39 @@ 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" + } + + /** Gets the corresponding flow label. */ + deprecated DataFlow::FlowLabel toFlowLabel() { + this = TTaint() and result.isTaint() + or + this = TObjectPrototype() and result instanceof ObjectPrototype + } + } + + /** 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. */ + 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 +63,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 +84,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 +110,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 +126,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..342c7e9cd931 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 state) { + node instanceof Source and state = FlowState::taint() } - predicate isSink(DataFlow::Node node, DataFlow::FlowLabel lbl) { - node.(Sink).getAFlowLabel() = lbl - } + predicate isSink(DataFlow::Node node, FlowState state) { node.(Sink).getAFlowState() = state } 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 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 instanceof ObjectPrototype + state = FlowState::objectPrototype() } - predicate isBarrierIn(DataFlow::Node node, DataFlow::FlowLabel 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, DataFlow::FlowLabel inlbl, DataFlow::Node succ, DataFlow::FlowLabel 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.isTaint() and - outlbl instanceof 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. @@ -94,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.isTaint() and - outlbl instanceof 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) @@ -105,22 +104,22 @@ module PrototypePollutingAssignmentConfig implements DataFlow::StateConfigSig { // DataFlow::localFieldStep(pred, succ) none() or - inlbl.isTaint() 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, DataFlow::FlowLabel lbl) { - lbl.isTaint() 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 instanceof ObjectPrototype + state = FlowState::objectPrototype() or - node = DataFlow::MakeLabeledBarrierGuard::getABarrierNode(lbl) + node = DataFlow::MakeStateBarrierGuard::getABarrierNode(state) } } @@ -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 state) { e = astNode.getBase() and outcome = true and - label instanceof 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, DataFlow::FlowLabel label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { e = astNode.getRightOperand() and outcome = true and - label instanceof 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, DataFlow::FlowLabel label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { e = astNode.getLeftOperand() and outcome = true and - label instanceof ObjectPrototype + state = 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 state) { polarity = outcome and e = operand and - label instanceof 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, DataFlow::FlowLabel label) { + override predicate blocksExpr(boolean outcome, Expr e, FlowState state) { e = this.getArgument(0).asExpr() and outcome = true and - label instanceof ObjectPrototype + state = FlowState::objectPrototype() } } 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..03d5e0c62a1b 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 node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - TaintedObject::step(src, dst, inlbl, outlbl) + TaintedObject::isAdditionalFlowStep(node1, state1, node2, state2) } 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) } } 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/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..16d15b42ce47 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/SecondOrderCommandInjectionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/SecondOrderCommandInjectionQuery.qll @@ -15,39 +15,37 @@ 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 node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - TaintedObject::step(src, trg, inlbl, 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/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..66e401d40ac1 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/TemplateObjectInjectionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/TemplateObjectInjectionQuery.qll @@ -15,35 +15,35 @@ 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 node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - TaintedObject::step(src, trg, inlbl, 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/UnsafeDynamicMethodAccessCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessCustomizations.qll index 3c5cc713e6eb..756efb5f3fb6 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessCustomizations.qll @@ -10,16 +10,50 @@ 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" + } + + /** Gets the corresponding flow label. */ + deprecated DataFlow::FlowLabel toFlowLabel() { + this = TTaint() and result.isTaint() + or + this = TUnsafeFunction() and result instanceof UnsafeFunction + } + } + + /** 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. */ + 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 +61,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 +75,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 +108,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..86a225f894a8 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessQuery.qll @@ -9,11 +9,11 @@ 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 +21,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 state) { + source.(Source).getAFlowState() = state } - predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - sink.(Sink).getFlowLabel() = label - } + predicate isSink(DataFlow::Node sink, FlowState state) { sink.(Sink).getAFlowState() = state } predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer @@ -38,46 +36,44 @@ module UnsafeDynamicMethodAccessConfig implements DataFlow::StateConfigSig { not StringConcatenation::isCoercion(node) } - predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel label) { + predicate isBarrier(DataFlow::Node node, FlowState state) { TaintTracking::defaultSanitizer(node) and - label.isTaint() + state = 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 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.isTaint() and - dstlabel = 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.isTaint() and - dstlabel = unsafeFunction() + node1 = proj.getASelector() and + node2 = proj and + state1 = FlowState::taint() and + state2 = FlowState::unsafeFunction() ) } predicate isAdditionalFlowStep( - DataFlow::Node src, DataFlow::FlowLabel srclabel, DataFlow::Node dst, - DataFlow::FlowLabel 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.isTaint() and - TaintTracking::defaultTaintStep(src, dst) and - srclabel = dstlabel + state1 = FlowState::taint() and + TaintTracking::defaultTaintStep(node1, node2) and + state1 = state2 } } @@ -93,11 +89,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 +113,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)) } } 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..5fdf3825405f 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 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::step(pred, succ, inlbl, outlbl) + TaintedObject::isAdditionalFlowStep(node1, state1, node2, state2) or // property read from a tainted object is considered tainted - succ.(DataFlow::PropRead).getBase() = pred and - inlbl = TaintedObject::label() and - outlbl = DataFlow::FlowLabel::taint() + 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/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. */ 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/UnvalidatedDynamicMethodCallCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll index 00542294d887..f148ba2c96bb 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll @@ -10,16 +10,62 @@ 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" + } + + /** Gets the corresponding flow label. */ + 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 { + /** Gets the flow state corresponding to `label`. */ + 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 +73,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 +86,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 +116,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 +145,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 +153,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 +186,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 +207,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..d8b29fca9014 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 state) { + source.(Source).getAFlowState() = state } - predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - sink.(Sink).getFlowLabel() = label - } + predicate isSink(DataFlow::Node sink, FlowState state) { sink.(Sink).getAFlowState() = state } - predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel label) { - node.(Sanitizer).getFlowLabel() = label + predicate isBarrier(DataFlow::Node node, FlowState state) { + node.(Sanitizer).getAFlowState() = state or TaintTracking::defaultSanitizer(node) and - label.isTaint() + state = FlowState::taint() or - node = DataFlow::MakeLabeledBarrierGuard::getABarrierNode(label) + node = DataFlow::MakeStateBarrierGuard::getABarrierNode(state) } predicate isBarrier(DataFlow::Node node) { @@ -52,34 +50,33 @@ module UnvalidatedDynamicMethodCallConfig implements DataFlow::StateConfigSig { } predicate isAdditionalFlowStep( - DataFlow::Node src, DataFlow::FlowLabel srclabel, DataFlow::Node dst, - DataFlow::FlowLabel dstlabel + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { exists(DataFlow::PropRead read | - src = read.getPropertyNameExpr().flow() and - dst = read and - srclabel.isTaint() and + node1 = read.getPropertyNameExpr().flow() and + node2 = read and + state1 = FlowState::taint() and ( - dstlabel instanceof MaybeNonFunction + state2 = FlowState::maybeNonFunction() or // a property of `Object.create(null)` cannot come from a prototype not PropertyInjection::isPrototypeLessObject(read.getBase().getALocalSource()) and - dstlabel instanceof 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.isTaint() and - dstlabel instanceof MaybeNonFunction + state1 = FlowState::taint() and + state2 = FlowState::maybeNonFunction() or - srclabel.isTaint() and - TaintTracking::defaultTaintStep(src, dst) and - srclabel = dstlabel + state1 = FlowState::taint() and + TaintTracking::defaultTaintStep(node1, node2) and + state1 = state2 } } @@ -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)) } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/XssThroughDomQuery.qll index 086cbe1abf06..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) } } @@ -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 } } 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/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" 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" } } 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..f0734b877c96 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 @@ -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 state) { + source instanceof ActiveThreatModelSource and state = 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 state) { + sink instanceof WorkerThreads and state = 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 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.isDataOrTaint() and - succlbl instanceof UrlConstructorLabel + 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) ) } } 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 } } 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, _) 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/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() ) } } 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 { 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() ) } } 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 ) } }