Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -36,6 +36,36 @@ module MakeBarrierGuard<BarrierGuardSig BaseGuard> {
}
}

/**
* Provides access to barrier guards defined via models-as-data.
*/
module ExternalBarrierGuard {
private predicate guardCheck(DataFlow::Node g, Expr e, boolean branch, string kind) {
exists(API::CallNode call, API::Node parameter |
parameter = call.getAParameter() and
parameter = ModelOutput::getABarrierGuardNode(kind, branch)
|
g = call and
e = parameter.asSink().asExpr()
)
}

private class BarrierGuard extends DataFlow::Node {
BarrierGuard() { guardCheck(this, _, _, _) }

predicate blocksExpr(boolean outcome, Expr e, string kind) {
guardCheck(this, e, outcome, kind)
}
}

/**
* Gets a barrier guard node of the given `kind` defined via models-as-data.
*/
DataFlow::Node getAnExternalBarrierNode(string kind) {
result = MakeStateBarrierGuard<string, BarrierGuard>::getABarrierNode(kind)
}
}

deprecated private module DeprecationWrapper {
signature class LabeledBarrierGuardSig extends DataFlow::Node {
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ module CredentialsExpr {
private class CredentialsFromModel extends CredentialsNode {
string kind;

CredentialsFromModel() { this = ModelOutput::getASinkNode("credentials-" + kind).asSink() }
CredentialsFromModel() { ModelOutput::sinkNode(this, "credentials-" + kind) }

override string getCredentialsKind() { result = CredentialsExpr::normalizeKind(kind) }
}
4 changes: 2 additions & 2 deletions javascript/ql/lib/semmle/javascript/frameworks/NoSQL.qll
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module NoSql {
}

private class QueryFromModel extends Query {
QueryFromModel() { this = ModelOutput::getASinkNode("nosql-injection").asSink() }
QueryFromModel() { ModelOutput::sinkNode(this, "nosql-injection") }
}
}

Expand Down Expand Up @@ -46,7 +46,7 @@ private module MongoDB {

override DataFlow::Node getAQueryArgument() {
result = [this.getAnArgument(), this.getOptionArgument(_, _)] and
result = ModelOutput::getASinkNode("mongodb.sink").asSink()
ModelOutput::sinkNode(result, "mongodb.sink")
}

override DataFlow::Node getAResult() {
Expand Down
2 changes: 1 addition & 1 deletion javascript/ql/lib/semmle/javascript/frameworks/SQL.qll
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ module SQL {
abstract class SqlString extends DataFlow::Node { }

private class SqlStringFromModel extends SqlString {
SqlStringFromModel() { this = ModelOutput::getASinkNode("sql-injection").asSink() }
SqlStringFromModel() { ModelOutput::sinkNode(this, "sql-injection") }
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import Shared::ModelOutput as ModelOutput
* A remote flow source originating from a MaD source row.
*/
private class RemoteFlowSourceFromMaD extends RemoteFlowSource {
RemoteFlowSourceFromMaD() { this = ModelOutput::getASourceNode("remote").asSource() }
RemoteFlowSourceFromMaD() { ModelOutput::sourceNode(this, "remote") }

override string getSourceType() { result = "Remote flow" }
}
Expand All @@ -39,9 +39,9 @@ private class RemoteFlowSourceFromMaD extends RemoteFlowSource {
* A threat-model flow source originating from a data extension.
*/
private class ThreatModelSourceFromDataExtension extends ThreatModelSource::Range {
ThreatModelSourceFromDataExtension() { this = ModelOutput::getASourceNode(_).asSource() }
ThreatModelSourceFromDataExtension() { ModelOutput::sourceNode(this, _) }

override string getThreatModel() { this = ModelOutput::getASourceNode(result).asSource() }
override string getThreatModel() { ModelOutput::sourceNode(this, result) }

override string getSourceType() {
result = "Source node (" + this.getThreatModel() + ") [from data-extension]"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,26 @@ private predicate sinkModel(string type, string path, string kind, string model)
)
}

/** Holds if a barrier model exists for the given parameters. */
private predicate barrierModel(string type, string path, string kind, string model) {
// No deprecation adapter for barrier models, they were not around back then.
exists(QlBuiltins::ExtensionId madId |
Extensions::barrierModel(type, path, kind, madId) and
model = "MaD:" + madId.toString()
)
}

/** Holds if a barrier guard model exists for the given parameters. */
private predicate barrierGuardModel(
string type, string path, string branch, string kind, string model
) {
// No deprecation adapter for barrier models, they were not around back then.
exists(QlBuiltins::ExtensionId madId |
Extensions::barrierGuardModel(type, path, branch, kind, madId) and
model = "MaD:" + madId.toString()
)
}

/** Holds if a summary model `row` exists for the given parameters. */
private predicate summaryModel(
string type, string path, string input, string output, string kind, string model
Expand Down Expand Up @@ -400,6 +420,8 @@ predicate isRelevantType(string type) {
(
sourceModel(type, _, _, _) or
sinkModel(type, _, _, _) or
barrierModel(type, _, _, _) or
barrierGuardModel(type, _, _, _, _) or
summaryModel(type, _, _, _, _, _) or
typeModel(_, type, _)
) and
Expand Down Expand Up @@ -427,6 +449,8 @@ predicate isRelevantFullPath(string type, string path) {
(
sourceModel(type, path, _, _) or
sinkModel(type, path, _, _) or
barrierModel(type, path, _, _) or
barrierGuardModel(type, path, _, _, _) or
summaryModel(type, path, _, _, _, _) or
typeModel(_, type, path)
)
Expand Down Expand Up @@ -745,6 +769,32 @@ module ModelOutput {
)
}

/**
* Holds if a barrier model contributed `barrier` with the given `kind`.
*/
cached
API::Node getABarrierNode(string kind, string model) {
exists(string type, string path |
barrierModel(type, path, kind, model) and
result = getNodeFromPath(type, path)
)
}

/**
* Holds if a barrier model contributed `barrier` with the given `kind` for the given `branch`.
*/
cached
API::Node getABarrierGuardNode(string kind, boolean branch, string model) {
exists(string type, string path, string branch_str |
branch = true and branch_str = "true"
or
branch = false and branch_str = "false"
|
barrierGuardModel(type, path, branch_str, kind, model) and
result = getNodeFromPath(type, path)
)
}

/**
* Holds if a relevant summary exists for these parameters.
*/
Expand Down Expand Up @@ -787,15 +837,46 @@ module ModelOutput {
private import codeql.mad.ModelValidation as SharedModelVal

/**
* Holds if a CSV source model contributed `source` with the given `kind`.
* Holds if an external model contributed `source` with the given `kind`.
*/
API::Node getASourceNode(string kind) { result = getASourceNode(kind, _) }

/**
* Holds if a CSV sink model contributed `sink` with the given `kind`.
* Holds if an external model contributed `sink` with the given `kind`.
*/
API::Node getASinkNode(string kind) { result = getASinkNode(kind, _) }

/**
* Holds if an external model contributed `barrier` with the given `kind`.
*/
API::Node getABarrierNode(string kind) { result = getABarrierNode(kind, _) }

/**
* Holds if an external model contributed `barrier-guard` with the given `kind` and `branch`.
*/
API::Node getABarrierGuardNode(string kind, boolean branch) {
result = getABarrierGuardNode(kind, branch, _)
}

/**
* Holds if `node` is specified as a source with the given kind in an external model.
*/
predicate sourceNode(DataFlow::Node node, string kind) { node = getASourceNode(kind).asSource() }

/**
* Holds if `node` is specified as a sink with the given kind in an external model.
*/
predicate sinkNode(DataFlow::Node node, string kind) { node = getASinkNode(kind).asSink() }

/**
* Holds if `node` is specified as a barrier with the given kind in an external model.
*/
predicate barrierNode(DataFlow::Node node, string kind) {
node = getABarrierNode(kind).asSink()
or
node = DataFlow::ExternalBarrierGuard::getAnExternalBarrierNode(kind)
}

private module KindValConfig implements SharedModelVal::KindValidationConfigSig {
predicate summaryKind(string kind) { summaryModel(_, _, _, _, kind, _) }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,26 @@ extensible predicate sourceModel(
*/
extensible predicate sinkModel(string type, string path, string kind, QlBuiltins::ExtensionId madId);

/**
* Holds if the value at `(type, path)` should be seen as a barrier
* of the given `kind` and `madId` is the data extension row number.
*/
extensible predicate barrierModel(
string type, string path, string kind, QlBuiltins::ExtensionId madId
);

/**
* Holds if the value at `(type, path)` should be seen as a barrier guard
* of the given `kind` and `madId` is the data extension row number.
* `path` is assumed to lead to a parameter of a call (possibly `self`), and
* the call is guarding the parameter.
* `branch` is either `true` or `false`, indicating which branch of the guard
* is protecting the parameter.
*/
extensible predicate barrierGuardModel(
string type, string path, string branch, string kind, QlBuiltins::ExtensionId madId
);

/**
* Holds if in calls to `(type, path)`, the value referred to by `input`
* can flow to the value referred to by `output` and `madId` is the data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@ extensions:
extensible: summaryModel
data: []

- addsTo:
pack: codeql/javascript-all
extensible: barrierModel
data: []

- addsTo:
pack: codeql/javascript-all
extensible: barrierGuardModel
data: []

- addsTo:
pack: codeql/javascript-all
extensible: neutralModel
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ module CorsPermissiveConfiguration {
* The value of cors origin when initializing the application.
*/
class CorsOriginSink extends Sink, DataFlow::ValueNode {
CorsOriginSink() { this = ModelOutput::getASinkNode("cors-origin").asSink() }
CorsOriginSink() { ModelOutput::sinkNode(this, "cors-origin") }
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,6 @@ module ClientSideUrlRedirect {
}

private class SinkFromModel extends Sink {
SinkFromModel() { this = ModelOutput::getASinkNode("url-redirection").asSink() }
SinkFromModel() { ModelOutput::sinkNode(this, "url-redirection") }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,6 @@ module CodeInjection {
class JsonStringifySanitizer extends Sanitizer, JsonStringifyCall { }

private class SinkFromModel extends Sink {
SinkFromModel() { this = ModelOutput::getASinkNode("code-injection").asSink() }
SinkFromModel() { ModelOutput::sinkNode(this, "code-injection") }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ module CommandInjection {
}

private class SinkFromModel extends Sink {
SinkFromModel() { this = ModelOutput::getASinkNode("command-injection").asSink() }
SinkFromModel() { ModelOutput::sinkNode(this, "command-injection") }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,6 @@ module DomBasedXss {
}

private class SinkFromModel extends Sink {
SinkFromModel() { this = ModelOutput::getASinkNode("html-injection").asSink() }
SinkFromModel() { ModelOutput::sinkNode(this, "html-injection") }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,5 +86,5 @@ class JsonStringifySanitizer extends Sanitizer {
}

private class SinkFromModel extends Sink {
SinkFromModel() { this = ModelOutput::getASinkNode("log-injection").asSink() }
SinkFromModel() { ModelOutput::sinkNode(this, "log-injection") }
}
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,6 @@ module ReflectedXss {
}

private class SinkFromModel extends Sink {
SinkFromModel() { this = ModelOutput::getASinkNode("html-injection").asSink() }
SinkFromModel() { ModelOutput::sinkNode(this, "html-injection") }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ module RequestForgery {
}

private class SinkFromModel extends Sink {
SinkFromModel() { this = ModelOutput::getASinkNode("request-forgery").asSink() }
SinkFromModel() { ModelOutput::sinkNode(this, "request-forgery") }

override DataFlow::Node getARequest() { result = this }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,6 @@ module ServerSideUrlRedirect {
}

private class SinkFromModel extends Sink {
SinkFromModel() { this = ModelOutput::getASinkNode("url-redirection").asSink() }
SinkFromModel() { ModelOutput::sinkNode(this, "url-redirection") }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1121,6 +1121,6 @@ module TaintedPath {
}

private class SinkFromModel extends Sink {
SinkFromModel() { this = ModelOutput::getASinkNode("path-injection").asSink() }
SinkFromModel() { ModelOutput::sinkNode(this, "path-injection") }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,6 @@ module UnsafeDeserialization {
}

private class SinkFromModel extends Sink {
SinkFromModel() { this = ModelOutput::getASinkNode("unsafe-deserialization").asSink() }
SinkFromModel() { ModelOutput::sinkNode(this, "unsafe-deserialization") }
}
}
8 changes: 3 additions & 5 deletions javascript/ql/test/library-tests/frameworks/data/test.ql
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ module TestConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
source.(DataFlow::CallNode).getCalleeName() = "source"
or
source = ModelOutput::getASourceNode("test-source").asSource()
ModelOutput::sourceNode(source, "test-source")
}

predicate isSink(DataFlow::Node sink) {
sink = any(DataFlow::CallNode call | call.getCalleeName() = "sink").getAnArgument()
or
sink = ModelOutput::getASinkNode("test-sink").asSink()
ModelOutput::sinkNode(sink, "test-sink")
}
}

Expand All @@ -48,9 +48,7 @@ query predicate taintFlow(DataFlow::Node source, DataFlow::Node sink) {
TestFlow::flow(source, sink)
}

query predicate isSink(DataFlow::Node node, string kind) {
node = ModelOutput::getASinkNode(kind).asSink()
}
query predicate isSink(DataFlow::Node node, string kind) { ModelOutput::sinkNode(node, kind) }

query predicate syntaxErrors(ApiGraphModels::AccessPath path) { path.hasSyntaxError() }

Expand Down
Loading
Loading