Skip to content

Commit 2bd4cce

Browse files
authored
Merge pull request #21004 from yoff/python/mad-barriers
Python: MaD barriers
2 parents 793d2c7 + 55abc52 commit 2bd4cce

File tree

60 files changed

+652
-127
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

60 files changed

+652
-127
lines changed

javascript/ql/lib/semmle/javascript/dataflow/internal/BarrierGuards.qll

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,41 @@ module MakeBarrierGuard<BarrierGuardSig BaseGuard> {
3636
}
3737
}
3838

39+
/**
40+
* Provides access to barrier guards defined via models-as-data.
41+
*/
42+
module ExternalBarrierGuard {
43+
private predicate guardCheck(DataFlow::Node g, Expr e, boolean branch, string kind) {
44+
exists(API::CallNode call, API::Node parameter |
45+
parameter = call.getAParameter() and
46+
parameter = ModelOutput::getABarrierGuardNode(kind, branch)
47+
|
48+
g = call and
49+
e = parameter.asSink().asExpr()
50+
)
51+
}
52+
53+
private class BarrierGuard extends DataFlow::Node {
54+
BarrierGuard() { guardCheck(this, _, _, _) }
55+
56+
predicate blocksExpr(boolean outcome, Expr e, string kind) {
57+
guardCheck(this, e, outcome, kind)
58+
}
59+
}
60+
61+
/**
62+
* Gets a barrier guard node of the given `kind` defined via models-as-data.
63+
*
64+
* This only provides external barrier nodes defined as guards. To get all externally defined barrer nodes,
65+
* use `ModelOutput::barrierNode(node, kind)`.
66+
*
67+
* INTERNAL: Do not use.
68+
*/
69+
DataFlow::Node getAnExternalBarrierNode(string kind) {
70+
result = MakeStateBarrierGuard<string, BarrierGuard>::getABarrierNode(kind)
71+
}
72+
}
73+
3974
deprecated private module DeprecationWrapper {
4075
signature class LabeledBarrierGuardSig extends DataFlow::Node {
4176
/**

javascript/ql/lib/semmle/javascript/frameworks/Credentials.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ module CredentialsExpr {
2929
private class CredentialsFromModel extends CredentialsNode {
3030
string kind;
3131

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

3434
override string getCredentialsKind() { result = CredentialsExpr::normalizeKind(kind) }
3535
}

javascript/ql/lib/semmle/javascript/frameworks/NoSQL.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ module NoSql {
1313
}
1414

1515
private class QueryFromModel extends Query {
16-
QueryFromModel() { this = ModelOutput::getASinkNode("nosql-injection").asSink() }
16+
QueryFromModel() { ModelOutput::sinkNode(this, "nosql-injection") }
1717
}
1818
}
1919

@@ -46,7 +46,7 @@ private module MongoDB {
4646

4747
override DataFlow::Node getAQueryArgument() {
4848
result = [this.getAnArgument(), this.getOptionArgument(_, _)] and
49-
result = ModelOutput::getASinkNode("mongodb.sink").asSink()
49+
ModelOutput::sinkNode(result, "mongodb.sink")
5050
}
5151

5252
override DataFlow::Node getAResult() {

javascript/ql/lib/semmle/javascript/frameworks/NodeJSLib.model.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,9 @@ extensions:
88
- ['global', 'Member[process].Member[stdin].Member[on,addListener].WithStringArgument[0=data].Argument[1].Parameter[0]', 'stdin']
99
- ['readline', 'Member[createInterface].ReturnValue.Member[question].Argument[1].Parameter[0]', 'stdin']
1010
- ['readline', 'Member[createInterface].ReturnValue.Member[on,addListener].WithStringArgument[0=line].Argument[1].Parameter[0]', 'stdin']
11+
12+
- addsTo:
13+
pack: codeql/javascript-all
14+
extensible: barrierModel
15+
data:
16+
- ['global', 'Member[encodeURIComponent,encodeURI].ReturnValue', 'request-forgery']

javascript/ql/lib/semmle/javascript/frameworks/SQL.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ module SQL {
99
abstract class SqlString extends DataFlow::Node { }
1010

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

1515
/**

javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import Shared::ModelOutput as ModelOutput
3030
* A remote flow source originating from a MaD source row.
3131
*/
3232
private class RemoteFlowSourceFromMaD extends RemoteFlowSource {
33-
RemoteFlowSourceFromMaD() { this = ModelOutput::getASourceNode("remote").asSource() }
33+
RemoteFlowSourceFromMaD() { ModelOutput::sourceNode(this, "remote") }
3434

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

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

4646
override string getSourceType() {
4747
result = "Source node (" + this.getThreatModel() + ") [from data-extension]"

javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll

Lines changed: 87 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,26 @@ private predicate sinkModel(string type, string path, string kind, string model)
344344
)
345345
}
346346

347+
/** Holds if a barrier model exists for the given parameters. */
348+
private predicate barrierModel(string type, string path, string kind, string model) {
349+
// No deprecation adapter for barrier models, they were not around back then.
350+
exists(QlBuiltins::ExtensionId madId |
351+
Extensions::barrierModel(type, path, kind, madId) and
352+
model = "MaD:" + madId.toString()
353+
)
354+
}
355+
356+
/** Holds if a barrier guard model exists for the given parameters. */
357+
private predicate barrierGuardModel(
358+
string type, string path, string branch, string kind, string model
359+
) {
360+
// No deprecation adapter for barrier models, they were not around back then.
361+
exists(QlBuiltins::ExtensionId madId |
362+
Extensions::barrierGuardModel(type, path, branch, kind, madId) and
363+
model = "MaD:" + madId.toString()
364+
)
365+
}
366+
347367
/** Holds if a summary model `row` exists for the given parameters. */
348368
private predicate summaryModel(
349369
string type, string path, string input, string output, string kind, string model
@@ -400,6 +420,8 @@ predicate isRelevantType(string type) {
400420
(
401421
sourceModel(type, _, _, _) or
402422
sinkModel(type, _, _, _) or
423+
barrierModel(type, _, _, _) or
424+
barrierGuardModel(type, _, _, _, _) or
403425
summaryModel(type, _, _, _, _, _) or
404426
typeModel(_, type, _)
405427
) and
@@ -427,6 +449,8 @@ predicate isRelevantFullPath(string type, string path) {
427449
(
428450
sourceModel(type, path, _, _) or
429451
sinkModel(type, path, _, _) or
452+
barrierModel(type, path, _, _) or
453+
barrierGuardModel(type, path, _, _, _) or
430454
summaryModel(type, path, _, _, _, _) or
431455
typeModel(_, type, path)
432456
)
@@ -747,6 +771,32 @@ module ModelOutput {
747771
)
748772
}
749773

774+
/**
775+
* Holds if a barrier model contributed `barrier` with the given `kind`.
776+
*/
777+
cached
778+
API::Node getABarrierNode(string kind, string model) {
779+
exists(string type, string path |
780+
barrierModel(type, path, kind, model) and
781+
result = getNodeFromPath(type, path)
782+
)
783+
}
784+
785+
/**
786+
* Holds if a barrier model contributed `barrier` with the given `kind` for the given `branch`.
787+
*/
788+
cached
789+
API::Node getABarrierGuardNode(string kind, boolean branch, string model) {
790+
exists(string type, string path, string branch_str |
791+
branch = true and branch_str = "true"
792+
or
793+
branch = false and branch_str = "false"
794+
|
795+
barrierGuardModel(type, path, branch_str, kind, model) and
796+
result = getNodeFromPath(type, path)
797+
)
798+
}
799+
750800
/**
751801
* Holds if a relevant summary exists for these parameters.
752802
*/
@@ -789,15 +839,50 @@ module ModelOutput {
789839
private import codeql.mad.ModelValidation as SharedModelVal
790840

791841
/**
792-
* Holds if a CSV source model contributed `source` with the given `kind`.
842+
* Holds if an external model contributed `source` with the given `kind`.
793843
*/
794844
API::Node getASourceNode(string kind) { result = getASourceNode(kind, _) }
795845

796846
/**
797-
* Holds if a CSV sink model contributed `sink` with the given `kind`.
847+
* Holds if an external model contributed `sink` with the given `kind`.
798848
*/
799849
API::Node getASinkNode(string kind) { result = getASinkNode(kind, _) }
800850

851+
/**
852+
* Holds if an external model contributed `barrier` with the given `kind`.
853+
*
854+
* INTERNAL: Do not use.
855+
*/
856+
API::Node getABarrierNode(string kind) { result = getABarrierNode(kind, _) }
857+
858+
/**
859+
* Holds if an external model contributed `barrier-guard` with the given `kind` and `branch`.
860+
*
861+
* INTERNAL: Do not use.
862+
*/
863+
API::Node getABarrierGuardNode(string kind, boolean branch) {
864+
result = getABarrierGuardNode(kind, branch, _)
865+
}
866+
867+
/**
868+
* Holds if `node` is specified as a source with the given kind in an external model.
869+
*/
870+
predicate sourceNode(DataFlow::Node node, string kind) { node = getASourceNode(kind).asSource() }
871+
872+
/**
873+
* Holds if `node` is specified as a sink with the given kind in an external model.
874+
*/
875+
predicate sinkNode(DataFlow::Node node, string kind) { node = getASinkNode(kind).asSink() }
876+
877+
/**
878+
* Holds if `node` is specified as a barrier with the given kind in an external model.
879+
*/
880+
predicate barrierNode(DataFlow::Node node, string kind) {
881+
node = getABarrierNode(kind).asSource()
882+
or
883+
node = DataFlow::ExternalBarrierGuard::getAnExternalBarrierNode(kind)
884+
}
885+
801886
private module KindValConfig implements SharedModelVal::KindValidationConfigSig {
802887
predicate summaryKind(string kind) { summaryModel(_, _, _, _, kind, _) }
803888

javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExtensions.qll

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,26 @@ extensible predicate sourceModel(
2020
*/
2121
extensible predicate sinkModel(string type, string path, string kind, QlBuiltins::ExtensionId madId);
2222

23+
/**
24+
* Holds if the value at `(type, path)` should be seen as a barrier
25+
* of the given `kind` and `madId` is the data extension row number.
26+
*/
27+
extensible predicate barrierModel(
28+
string type, string path, string kind, QlBuiltins::ExtensionId madId
29+
);
30+
31+
/**
32+
* Holds if the value at `(type, path)` should be seen as a barrier guard
33+
* of the given `kind` and `madId` is the data extension row number.
34+
* `path` is assumed to lead to a parameter of a call (possibly `self`), and
35+
* the call is guarding the parameter.
36+
* `branch` is either `true` or `false`, indicating which branch of the guard
37+
* is protecting the parameter.
38+
*/
39+
extensible predicate barrierGuardModel(
40+
string type, string path, string branch, string kind, QlBuiltins::ExtensionId madId
41+
);
42+
2343
/**
2444
* Holds if in calls to `(type, path)`, the value referred to by `input`
2545
* can flow to the value referred to by `output` and `madId` is the data

javascript/ql/lib/semmle/javascript/frameworks/data/internal/empty.model.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,16 @@ extensions:
1616
extensible: summaryModel
1717
data: []
1818

19+
- addsTo:
20+
pack: codeql/javascript-all
21+
extensible: barrierModel
22+
data: []
23+
24+
- addsTo:
25+
pack: codeql/javascript-all
26+
extensible: barrierGuardModel
27+
data: []
28+
1929
- addsTo:
2030
pack: codeql/javascript-all
2131
extensible: neutralModel

javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ module CorsPermissiveConfiguration {
6666
* The value of cors origin when initializing the application.
6767
*/
6868
class CorsOriginSink extends Sink, DataFlow::ValueNode {
69-
CorsOriginSink() { this = ModelOutput::getASinkNode("cors-origin").asSink() }
69+
CorsOriginSink() { ModelOutput::sinkNode(this, "cors-origin") }
7070
}
7171

7272
/**

0 commit comments

Comments
 (0)