Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JS: Migrate all queries to proper flow states and deprecate FlowLabel #18265

Merged
merged 37 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
a8fdd75
JS: Add FlowState class to TaintedUrlSuffix
asgerf Dec 11, 2024
cca9802
JS: Use flow state in barrier and step relations
asgerf Dec 11, 2024
3cf14d8
JS: Migrate ClientSideUrlRedirect to flow state
asgerf Dec 11, 2024
114d4a1
JS: Move FlowState definition into CommonFlowState
asgerf Dec 11, 2024
12289d4
JS: Migrate DomBasedXssQuery to FlowState
asgerf Dec 11, 2024
14ca1c1
JS: Update TaintedUrlSuffix test
asgerf Dec 11, 2024
5f42a71
JS: Migrate TaintedObject to a CommonFlowState
asgerf Dec 11, 2024
15d999a
JS: Migrate DeepObjectResourceExhaustion
asgerf Dec 11, 2024
daddff0
JS: Avoid deprecation warning in XssThroughDom
asgerf Dec 11, 2024
8e8de5c
JS: Migrate LoopBoundInjection
asgerf Dec 12, 2024
c38e3a2
JS: Migrate NoSqlInjection
asgerf Dec 12, 2024
355f7cd
JS: Migrate PrototypePollutingMergeCall
asgerf Dec 12, 2024
3573f0b
JS: Migrate SecondOrderCommandInjection
asgerf Dec 12, 2024
8907252
JS: Migrate TemplateObjectInjection
asgerf Dec 12, 2024
d9a43db
JS: Migrate UnsafeHtmlConstruction
asgerf Dec 12, 2024
42a7208
JS: Migrate ExceptionXss
asgerf Dec 12, 2024
dc3d7a0
Update ExceptionXssCustomizations.qll
asgerf Dec 13, 2024
2112ecc
JS: Migrate HardcodedDataInterpretedAsCode
asgerf Dec 13, 2024
d381ab1
JS: Migrate IncompleteHtmlAttributeSanitization
asgerf Dec 13, 2024
4e25036
JS: Follow naming convention in InsecureModuleFlow module
asgerf Dec 13, 2024
bcc1669
JS: Migrate InsecureDownload
asgerf Dec 13, 2024
a9e89ed
JS: Migrate PrototypePollutingAssignment
asgerf Dec 13, 2024
820f81f
JS: Migrate UnsafeDynamicMethodAccess
asgerf Dec 13, 2024
c951a29
JS: Migrate UnvalidatedDynamicMethodCall
asgerf Dec 13, 2024
a398599
JS: Rename an experimental query
asgerf Dec 13, 2024
d83ddfa
JS: Migrate an experimental CodeInjection query
asgerf Dec 13, 2024
ebe596f
JS: Migrate CorsPermissiveConfiguration
asgerf Dec 13, 2024
73af3f3
JS: Migrate PrototypePollutingFunction
asgerf Dec 13, 2024
69b361a
JS: Migrate a test to use flow state
asgerf Dec 13, 2024
d993c88
JS: Deprecate the FlowLabel class
asgerf Dec 13, 2024
ac6da6c
JS: Add some missing qldoc
asgerf Dec 13, 2024
079294e
JS: Mass rename to node1,state1,node2,state2 naming convention
asgerf Dec 13, 2024
cf6d166
JS: Also update tutorial code
asgerf Dec 13, 2024
db00dad
JS: Avoid deprecation warnings in some tests
asgerf Dec 13, 2024
0b2914f
JS: A few more deprecation updates
asgerf Dec 13, 2024
947b785
JS: Remove reference to deprecated step relation that's empty anyway
asgerf Dec 13, 2024
e5ae7e0
JS: Fix bad join in isOptionallySanitizedEdgeInternal
asgerf Dec 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
}
}
Expand All @@ -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
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
) {
Expand Down Expand Up @@ -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
) {
Expand Down Expand Up @@ -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
) {
Expand Down Expand Up @@ -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
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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".
Expand All @@ -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() }

Expand Down Expand Up @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,19 @@ module MakeBarrierGuard<BarrierGuardSig BaseGuard> {
}
}

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<LabeledBarrierGuardSig BaseGuard> {
deprecated module MakeLabeledBarrierGuard<DeprecationWrapper::LabeledBarrierGuardSig BaseGuard> {
final private class FinalBaseGuard = BaseGuard;

private class Adapter extends FinalBaseGuard {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
Expand Down Expand Up @@ -634,7 +634,7 @@ class PathSummary extends TPathSummary {
}
}

module PathSummary {
deprecated module PathSummary {
/**
* Gets a summary describing a path without any calls or returns.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -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 }
}
Loading
Loading