Skip to content

Commit 3e8f32c

Browse files
NapalysNapalys
committed
Merged CorsPermissiveConfig to CorsMisconfigurationForCredentials
1 parent 3a4a57f commit 3e8f32c

File tree

5 files changed

+102
-17
lines changed

5 files changed

+102
-17
lines changed

‎javascript/ql/lib/semmle/javascript/security/dataflow/CorsMisconfigurationForCredentialsCustomizations.qll

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
/**
22
* Provides default sources, sinks and sanitizers for reasoning about
3-
* CORS misconfiguration for credentials transfer, as well as
4-
* extension points for adding your own.
3+
* CORS misconfiguration for credentials transfer and overly permissive CORS configurations,
4+
* as well as extension points for adding your own.
55
*/
66

77
import javascript
8+
private import semmle.javascript.frameworks.Cors
89

910
module CorsMisconfigurationForCredentials {
1011
/**
11-
* A data flow source for CORS misconfiguration for credentials transfer.
12+
* A data flow source for CORS misconfiguration for credentials transfer and overly permissive CORS configurations.
1213
*/
1314
abstract class Source extends DataFlow::Node { }
1415

@@ -75,4 +76,58 @@ module CorsMisconfigurationForCredentials {
7576
this.asExpr().mayHaveStringValue("null")
7677
}
7778
}
79+
80+
/** An overly permissive value for `origin` (Apollo) */
81+
class TrueNullValue extends Source {
82+
TrueNullValue() { this.mayHaveBooleanValue(true) or this.asExpr() instanceof NullLiteral }
83+
}
84+
85+
/** An overly permissive value for `origin` (Express) */
86+
class WildcardValue extends Source {
87+
WildcardValue() { this.mayHaveStringValue("*") }
88+
}
89+
90+
/**
91+
* The value of cors origin when initializing the application.
92+
*/
93+
class CorsApolloServer extends Sink, DataFlow::ValueNode {
94+
CorsApolloServer() {
95+
exists(API::NewNode agql |
96+
agql = ModelOutput::getATypeNode("ApolloServer").getAnInstantiation() and
97+
this =
98+
agql.getOptionArgument(0, "cors").getALocalSource().getAPropertyWrite("origin").getRhs()
99+
)
100+
}
101+
102+
override Http::HeaderDefinition getCredentialsHeader() { none() }
103+
}
104+
105+
/**
106+
* The value of cors origin when initializing the application.
107+
*/
108+
class ExpressCors extends Sink, DataFlow::ValueNode {
109+
ExpressCors() {
110+
exists(CorsConfiguration config | this = config.getCorsConfiguration().getOrigin())
111+
}
112+
113+
override Http::HeaderDefinition getCredentialsHeader() { none() }
114+
}
115+
116+
/**
117+
* An express route setup configured with the `cors` package.
118+
*/
119+
class CorsConfiguration extends DataFlow::MethodCallNode {
120+
Cors::Cors corsConfig;
121+
122+
CorsConfiguration() {
123+
exists(Express::RouteSetup setup | this = setup |
124+
if setup.isUseCall()
125+
then corsConfig = setup.getArgument(0)
126+
else corsConfig = setup.getArgument(any(int i | i > 0))
127+
)
128+
}
129+
130+
/** Gets the expression that configures `cors` on this route setup. */
131+
Cors::Cors getCorsConfiguration() { result = corsConfig }
132+
}
78133
}

‎javascript/ql/src/Security/CWE-346/CorsMisconfigurationForCredentials.ql

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
* @name CORS misconfiguration for credentials transfer
3-
* @description Misconfiguration of CORS HTTP headers allows for leaks of secret credentials.
3+
* @description Misconfiguration of CORS HTTP headers allows for leaks of secret credentials and CSRF attacks.
44
* @kind path-problem
55
* @problem.severity error
66
* @security-severity 7.5
@@ -18,6 +18,5 @@ import CorsMisconfigurationFlow::PathGraph
1818

1919
from CorsMisconfigurationFlow::PathNode source, CorsMisconfigurationFlow::PathNode sink
2020
where CorsMisconfigurationFlow::flowPath(source, sink)
21-
select sink.getNode(), source, sink, "$@ leak vulnerability due to a $@.",
22-
sink.getNode().(Sink).getCredentialsHeader(), "Credential", source.getNode(),
23-
"misconfigured CORS header value"
21+
select sink.getNode(), source, sink,
22+
"CORS misconfiguration due to a $@.", source.getNode(), "permissive or user controlled value"

‎javascript/ql/test/query-tests/Security/CWE-346/CorsMisconfigurationForCredentials.expected

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,46 @@
11
#select
2-
| tst.js:13:50:13:55 | origin | tst.js:12:28:12:34 | req.url | tst.js:13:50:13:55 | origin | $@ leak vulnerability due to a $@. | tst.js:14:5:14:59 | res.set ... , true) | Credential | tst.js:12:28:12:34 | req.url | misconfigured CORS header value |
3-
| tst.js:18:50:18:53 | null | tst.js:18:50:18:53 | null | tst.js:18:50:18:53 | null | $@ leak vulnerability due to a $@. | tst.js:19:5:19:59 | res.set ... , true) | Credential | tst.js:18:50:18:53 | null | misconfigured CORS header value |
4-
| tst.js:23:50:23:55 | "null" | tst.js:23:50:23:55 | "null" | tst.js:23:50:23:55 | "null" | $@ leak vulnerability due to a $@. | tst.js:24:5:24:59 | res.set ... , true) | Credential | tst.js:23:50:23:55 | "null" | misconfigured CORS header value |
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 misconfiguration due to a $@. | apollo-test.js:11:25:11:28 | true | 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 misconfiguration due to a $@. | apollo-test.js:21:25:21:28 | null | 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 misconfiguration due to a $@. | apollo-test.js:8:33:8:39 | req.url | 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 misconfiguration due to a $@. | apollo-test.js:8:42:8:45 | true | 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 misconfiguration due to a $@. | express-test.js:26:17:26:19 | '*' | 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 misconfiguration due to a $@. | express-test.js:10:33:10:39 | req.url | 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 misconfiguration due to a $@. | express-test.js:10:42:10:45 | true | permissive or user controlled value |
9+
| tst.js:13:50:13:55 | origin | tst.js:12:28:12:34 | req.url | tst.js:13:50:13:55 | origin | CORS misconfiguration due to a $@. | tst.js:12:28:12:34 | req.url | permissive or user controlled value |
10+
| tst.js:13:50:13:55 | origin | tst.js:12:37:12:40 | true | tst.js:13:50:13:55 | origin | CORS misconfiguration due to a $@. | tst.js:12:37:12:40 | true | permissive or user controlled value |
11+
| tst.js:18:50:18:53 | null | tst.js:18:50:18:53 | null | tst.js:18:50:18:53 | null | CORS misconfiguration due to a $@. | tst.js:18:50:18:53 | null | permissive or user controlled value |
12+
| tst.js:23:50:23:55 | "null" | tst.js:23:50:23:55 | "null" | tst.js:23:50:23:55 | "null" | CORS misconfiguration due to a $@. | tst.js:23:50:23:55 | "null" | permissive or user controlled value |
513
edges
14+
| apollo-test.js:8:9:8:59 | user_origin | apollo-test.js:26:25:26:35 | user_origin | provenance | |
15+
| apollo-test.js:8:23:8:46 | url.par ... , true) | apollo-test.js:8:9:8:59 | user_origin | provenance | |
16+
| apollo-test.js:8:33:8:39 | req.url | apollo-test.js:8:23:8:46 | url.par ... , true) | provenance | |
17+
| apollo-test.js:8:42:8:45 | true | apollo-test.js:8:23:8:46 | url.par ... , true) | provenance | |
18+
| express-test.js:10:9:10:59 | user_origin | express-test.js:33:17:33:27 | user_origin | provenance | |
19+
| express-test.js:10:23:10:46 | url.par ... , true) | express-test.js:10:9:10:59 | user_origin | provenance | |
20+
| 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 | |
622
| tst.js:12:9:12:54 | origin | tst.js:13:50:13:55 | origin | provenance | |
723
| tst.js:12:18:12:41 | url.par ... , true) | tst.js:12:9:12:54 | origin | provenance | |
824
| tst.js:12:28:12:34 | req.url | tst.js:12:18:12:41 | url.par ... , true) | provenance | |
25+
| tst.js:12:37:12:40 | true | tst.js:12:18:12:41 | url.par ... , true) | provenance | |
926
nodes
27+
| apollo-test.js:8:9:8:59 | user_origin | semmle.label | user_origin |
28+
| apollo-test.js:8:23:8:46 | url.par ... , true) | semmle.label | url.par ... , true) |
29+
| apollo-test.js:8:33:8:39 | req.url | semmle.label | req.url |
30+
| apollo-test.js:8:42:8:45 | true | semmle.label | true |
31+
| apollo-test.js:11:25:11:28 | true | semmle.label | true |
32+
| apollo-test.js:21:25:21:28 | null | semmle.label | null |
33+
| apollo-test.js:26:25:26:35 | 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) |
36+
| express-test.js:10:33:10:39 | req.url | semmle.label | req.url |
37+
| express-test.js:10:42:10:45 | true | semmle.label | true |
38+
| express-test.js:26:17:26:19 | '*' | semmle.label | '*' |
39+
| express-test.js:33:17:33:27 | user_origin | semmle.label | user_origin |
1040
| tst.js:12:9:12:54 | origin | semmle.label | origin |
1141
| tst.js:12:18:12:41 | url.par ... , true) | semmle.label | url.par ... , true) |
1242
| tst.js:12:28:12:34 | req.url | semmle.label | req.url |
43+
| tst.js:12:37:12:40 | true | semmle.label | true |
1344
| tst.js:13:50:13:55 | origin | semmle.label | origin |
1445
| tst.js:18:50:18:53 | null | semmle.label | null |
1546
| tst.js:23:50:23:55 | "null" | semmle.label | "null" |

‎javascript/ql/test/query-tests/Security/CWE-346/apollo-test.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ var https = require('https'),
55
var server = https.createServer(function () { });
66

77
server.on('request', function (req, res) {
8-
let user_origin = url.parse(req.url, true).query.origin; // $ MISSING: Source
8+
let user_origin = url.parse(req.url, true).query.origin; // $ Source
99
// BAD: CORS too permissive
1010
const server_1 = new ApolloServer({
11-
cors: { origin: true } // $ MISSING: Alert
11+
cors: { origin: true } // $ Alert
1212
});
1313

1414
// GOOD: restrictive CORS
@@ -18,11 +18,11 @@ server.on('request', function (req, res) {
1818

1919
// BAD: CORS too permissive
2020
const server_3 = new ApolloServer({
21-
cors: { origin: null } // $ MISSING: Alert
21+
cors: { origin: null } // $ Alert
2222
});
2323

2424
// BAD: CORS is controlled by user
2525
const server_4 = new ApolloServer({
26-
cors: { origin: user_origin } // $ MISSING: Alert
26+
cors: { origin: user_origin } // $ Alert
2727
});
2828
});

‎javascript/ql/test/query-tests/Security/CWE-346/express-test.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ var https = require('https'),
77
var server = https.createServer(function () { });
88

99
server.on('request', function (req, res) {
10-
let user_origin = url.parse(req.url, true).query.origin; // $ MISSING: Source
10+
let user_origin = url.parse(req.url, true).query.origin; // $ Source
1111

1212
// BAD: CORS too permissive, default value is *
1313
var app1 = express();
@@ -23,14 +23,14 @@ server.on('request', function (req, res) {
2323
// BAD: CORS too permissive
2424
var app3 = express();
2525
var corsOption3 = {
26-
origin: '*' // $ MISSING: Alert
26+
origin: '*' // $ Alert
2727
};
2828
app3.use(cors(corsOption3));
2929

3030
// BAD: CORS is controlled by user
3131
var app4 = express();
3232
var corsOption4 = {
33-
origin: user_origin // $ MISSING: Alert
33+
origin: user_origin // $ Alert
3434
};
3535
app4.use(cors(corsOption4));
3636
});

0 commit comments

Comments
 (0)