Skip to content

Commit 4dac80a

Browse files
NapalysNapalys
committed
Replace complex wrapper classes with MaD
1 parent 021aa13 commit 4dac80a

File tree

7 files changed

+27
-71
lines changed

7 files changed

+27
-71
lines changed

‎javascript/ql/lib/ext/apollo-server.model.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ extensions:
1010
extensible: sinkModel
1111
data:
1212
- ["@apollo/server", "Member[gql].Argument[0]", "sql-injection"]
13+
- ["@apollo/server", "Member[ApolloServer,ApolloServerBase].Argument[0].Member[cors].Member[origin]", "cors-misconfiguration"]
1314

1415
- addsTo:
1516
pack: codeql/javascript-all

‎javascript/ql/lib/ext/cors.model.yml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/javascript-all
4+
extensible: sinkModel
5+
data:
6+
- ["cors", "Argument[0].Member[origin]", "cors-misconfiguration"]

‎javascript/ql/lib/semmle/javascript/frameworks/Cors.qll‎

Lines changed: 0 additions & 24 deletions
This file was deleted.

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

Lines changed: 2 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
*/
66

77
import javascript
8-
private import semmle.javascript.frameworks.Cors
98

109
/** Module containing sources, sinks, and sanitizers for overly permissive CORS configurations. */
1110
module CorsPermissiveConfiguration {
@@ -73,40 +72,7 @@ module CorsPermissiveConfiguration {
7372
/**
7473
* The value of cors origin when initializing the application.
7574
*/
76-
class CorsApolloServer extends Sink, DataFlow::ValueNode {
77-
CorsApolloServer() {
78-
exists(API::NewNode agql |
79-
agql = ModelOutput::getATypeNode("ApolloServer").getAnInstantiation() and
80-
this =
81-
agql.getOptionArgument(0, "cors").getALocalSource().getAPropertyWrite("origin").getRhs()
82-
)
83-
}
84-
}
85-
86-
/**
87-
* The value of cors origin when initializing the application.
88-
*/
89-
class ExpressCors extends Sink, DataFlow::ValueNode {
90-
ExpressCors() {
91-
exists(CorsConfiguration config | this = config.getCorsConfiguration().getOrigin())
92-
}
93-
}
94-
95-
/**
96-
* An express route setup configured with the `cors` package.
97-
*/
98-
class CorsConfiguration extends DataFlow::MethodCallNode {
99-
Cors::Cors corsConfig;
100-
101-
CorsConfiguration() {
102-
exists(Express::RouteSetup setup | this = setup |
103-
if setup.isUseCall()
104-
then corsConfig = setup.getArgument(0)
105-
else corsConfig = setup.getArgument(any(int i | i > 0))
106-
)
107-
}
108-
109-
/** Gets the expression that configures `cors` on this route setup. */
110-
Cors::Cors getCorsConfiguration() { result = corsConfig }
75+
class CorsOriginSink extends Sink, DataFlow::ValueNode {
76+
CorsOriginSink() { this = ModelOutput::getASinkNode("cors-misconfiguration").asSink() }
11177
}
11278
}

‎javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,8 @@ module CorsPermissiveConfigurationConfig implements DataFlow::StateConfigSig {
2727
}
2828

2929
predicate isSink(DataFlow::Node sink, FlowState state) {
30-
sink instanceof CorsApolloServer and state = [FlowState::taint(), FlowState::trueOrNull()]
31-
or
32-
sink instanceof ExpressCors and state = [FlowState::taint(), FlowState::wildcard()]
30+
sink instanceof CorsOriginSink and
31+
state = [FlowState::taint(), FlowState::trueOrNull(), FlowState::wildcard()]
3332
}
3433

3534
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }

‎javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.expected‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,11 @@
1+
#select
2+
| apollo-test.js:11:25:11:28 | true | apollo-test.js:11:25:11:28 | true | apollo-test.js:11:25:11:28 | true | CORS Origin misconfiguration due to a $@. | apollo-test.js:11:25:11:28 | true | too permissive or user controlled value |
3+
| apollo-test.js:21:25:21:28 | null | apollo-test.js:21:25:21:28 | null | apollo-test.js:21:25:21:28 | null | CORS Origin misconfiguration due to a $@. | apollo-test.js:21:25:21:28 | null | too permissive or user controlled value |
4+
| apollo-test.js:26:25:26:35 | user_origin | apollo-test.js:8:33:8:39 | req.url | apollo-test.js:26:25:26:35 | user_origin | CORS Origin misconfiguration due to a $@. | apollo-test.js:8:33:8:39 | req.url | too permissive or user controlled value |
5+
| apollo-test.js:26:25:26:35 | user_origin | apollo-test.js:8:42:8:45 | true | apollo-test.js:26:25:26:35 | user_origin | CORS Origin misconfiguration due to a $@. | apollo-test.js:8:42:8:45 | true | too permissive or user controlled value |
6+
| express-test.js:26:17:26:19 | '*' | express-test.js:26:17:26:19 | '*' | express-test.js:26:17:26:19 | '*' | CORS Origin misconfiguration due to a $@. | express-test.js:26:17:26:19 | '*' | too permissive or user controlled value |
7+
| express-test.js:33:17:33:27 | user_origin | express-test.js:10:33:10:39 | req.url | express-test.js:33:17:33:27 | user_origin | CORS Origin misconfiguration due to a $@. | express-test.js:10:33:10:39 | req.url | too permissive or user controlled value |
8+
| express-test.js:33:17:33:27 | user_origin | express-test.js:10:42:10:45 | true | express-test.js:33:17:33:27 | user_origin | CORS Origin misconfiguration due to a $@. | express-test.js:10:42:10:45 | true | too permissive or user controlled value |
19
edges
210
| apollo-test.js:8:9:8:59 | user_origin | apollo-test.js:26:25:26:35 | user_origin | provenance | |
311
| apollo-test.js:8:9:8:59 | user_origin | apollo-test.js:26:25:26:35 | user_origin | provenance | |
@@ -6,8 +14,11 @@ edges
614
| apollo-test.js:8:33:8:39 | req.url | apollo-test.js:8:23:8:46 | url.par ... , true) | provenance | |
715
| apollo-test.js:8:42:8:45 | true | apollo-test.js:8:23:8:46 | url.par ... , true) | provenance | |
816
| express-test.js:10:9:10:59 | user_origin | express-test.js:33:17:33:27 | user_origin | provenance | |
17+
| express-test.js:10:9:10:59 | user_origin | express-test.js:33:17:33:27 | user_origin | provenance | |
18+
| express-test.js:10:23:10:46 | url.par ... , true) | express-test.js:10:9:10:59 | user_origin | provenance | |
919
| express-test.js:10:23:10:46 | url.par ... , true) | express-test.js:10:9:10:59 | user_origin | provenance | |
1020
| express-test.js:10:33:10:39 | req.url | express-test.js:10:23:10:46 | url.par ... , true) | provenance | |
21+
| express-test.js:10:42:10:45 | true | express-test.js:10:23:10:46 | url.par ... , true) | provenance | |
1122
nodes
1223
| apollo-test.js:8:9:8:59 | user_origin | semmle.label | user_origin |
1324
| apollo-test.js:8:9:8:59 | user_origin | semmle.label | user_origin |
@@ -20,15 +31,12 @@ nodes
2031
| apollo-test.js:26:25:26:35 | user_origin | semmle.label | user_origin |
2132
| apollo-test.js:26:25:26:35 | user_origin | semmle.label | user_origin |
2233
| express-test.js:10:9:10:59 | user_origin | semmle.label | user_origin |
34+
| express-test.js:10:9:10:59 | user_origin | semmle.label | user_origin |
35+
| express-test.js:10:23:10:46 | url.par ... , true) | semmle.label | url.par ... , true) |
2336
| express-test.js:10:23:10:46 | url.par ... , true) | semmle.label | url.par ... , true) |
2437
| express-test.js:10:33:10:39 | req.url | semmle.label | req.url |
38+
| express-test.js:10:42:10:45 | true | semmle.label | true |
2539
| express-test.js:26:17:26:19 | '*' | semmle.label | '*' |
2640
| express-test.js:33:17:33:27 | user_origin | semmle.label | user_origin |
41+
| express-test.js:33:17:33:27 | user_origin | semmle.label | user_origin |
2742
subpaths
28-
#select
29-
| apollo-test.js:11:25:11:28 | true | apollo-test.js:11:25:11:28 | true | apollo-test.js:11:25:11:28 | true | CORS Origin misconfiguration due to a $@. | apollo-test.js:11:25:11:28 | true | too permissive or user controlled value |
30-
| apollo-test.js:21:25:21:28 | null | apollo-test.js:21:25:21:28 | null | apollo-test.js:21:25:21:28 | null | CORS Origin misconfiguration due to a $@. | apollo-test.js:21:25:21:28 | null | too permissive or user controlled value |
31-
| apollo-test.js:26:25:26:35 | user_origin | apollo-test.js:8:33:8:39 | req.url | apollo-test.js:26:25:26:35 | user_origin | CORS Origin misconfiguration due to a $@. | apollo-test.js:8:33:8:39 | req.url | too permissive or user controlled value |
32-
| apollo-test.js:26:25:26:35 | user_origin | apollo-test.js:8:42:8:45 | true | apollo-test.js:26:25:26:35 | user_origin | CORS Origin misconfiguration due to a $@. | apollo-test.js:8:42:8:45 | true | too permissive or user controlled value |
33-
| express-test.js:26:17:26:19 | '*' | express-test.js:26:17:26:19 | '*' | express-test.js:26:17:26:19 | '*' | CORS Origin misconfiguration due to a $@. | express-test.js:26:17:26:19 | '*' | too permissive or user controlled value |
34-
| express-test.js:33:17:33:27 | user_origin | express-test.js:10:33:10:39 | req.url | express-test.js:33:17:33:27 | user_origin | CORS Origin misconfiguration due to a $@. | express-test.js:10:33:10:39 | req.url | too permissive or user controlled value |

‎shared/mad/codeql/mad/ModelValidation.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ module KindValidation<KindValidationConfigSig Config> {
3939
"response-splitting", "trust-boundary-violation", "template-injection", "url-forward",
4040
"xslt-injection",
4141
// JavaScript-only currently, but may be shared in the future
42-
"mongodb.sink",
42+
"cors-misconfiguration", "mongodb.sink",
4343
// Swift-only currently, but may be shared in the future
4444
"database-store", "format-string", "hash-iteration-count", "predicate-injection",
4545
"preferences-store", "tls-protocol-version", "transmission", "webview-fetch", "xxe",

0 commit comments

Comments
 (0)