Skip to content

Commit fd3089e

Browse files
erik-krogherik-krogh
authored
Merge pull request github#14342 from maikypedia/maikypedia/javascript-cors
JS: Add Permissive CORS query (CWE-942)
2 parents 72caadb + d0cf2a9 commit fd3089e

13 files changed

+447
-0
lines changed

‎javascript/ql/src/change-notes/2023-11-27-cors-permissive-configuarion.md

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added a new experimental query, `js/cors-misconfiguration`, which detects misconfigured CORS HTTP headers in the `cors` and `apollo` libraries.

‎javascript/ql/src/experimental/Security/CWE-942/Apollo.qll

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/**
2+
* Provides classes for working with Apollo GraphQL connectors.
3+
*/
4+
5+
import javascript
6+
7+
/** Provides classes modeling the apollo packages [@apollo/server](https://npmjs.com/package/@apollo/server`) */
8+
module Apollo {
9+
/** Get a reference to the `ApolloServer` class. */
10+
private API::Node apollo() {
11+
result =
12+
API::moduleImport([
13+
"@apollo/server", "@apollo/apollo-server-express", "@apollo/apollo-server-core",
14+
"apollo-server", "apollo-server-express"
15+
]).getMember("ApolloServer")
16+
}
17+
18+
/** Gets a reference to the `gql` function that parses GraphQL strings. */
19+
private API::Node gql() {
20+
result =
21+
API::moduleImport([
22+
"@apollo/server", "@apollo/apollo-server-express", "@apollo/apollo-server-core",
23+
"apollo-server", "apollo-server-express"
24+
]).getMember("gql")
25+
}
26+
27+
/** An instantiation of an `ApolloServer`. */
28+
class ApolloServer extends API::NewNode {
29+
ApolloServer() { this = apollo().getAnInstantiation() }
30+
}
31+
32+
/** A string that is interpreted as a GraphQL query by a `apollo` package. */
33+
private class ApolloGraphQLString extends GraphQL::GraphQLString {
34+
ApolloGraphQLString() { this = gql().getACall().getArgument(0) }
35+
}
36+
}

‎javascript/ql/src/experimental/Security/CWE-942/Cors.qll

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* Provides classes for working with Cors connectors.
3+
*/
4+
5+
import javascript
6+
7+
/** Provides classes modeling the [cors](https://npmjs.com/package/cors) library. */
8+
module Cors {
9+
/**
10+
* An expression that creates a new CORS configuration.
11+
*/
12+
class Cors extends DataFlow::CallNode {
13+
Cors() { this = DataFlow::moduleImport("cors").getAnInvocation() }
14+
15+
/** Get the options used to configure Cors */
16+
DataFlow::Node getOptionsArgument() { result = this.getArgument(0) }
17+
18+
/** Holds if cors is using default configuration */
19+
predicate isDefault() { this.getNumArgument() = 0 }
20+
21+
/** Gets the value of the `origin` option used to configure this Cors instance. */
22+
DataFlow::Node getOrigin() { result = this.getOptionArgument(0, "origin") }
23+
}
24+
}

‎javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfiguration.qhelp

+71
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
9+
A server can use <code>CORS</code> (Cross-Origin Resource Sharing) to relax the
10+
restrictions imposed by the <code>SOP</code> (Same-Origin Policy), allowing controlled, secure
11+
cross-origin requests when necessary.
12+
13+
A server with an overly permissive <code>CORS</code> configuration may inadvertently
14+
expose sensitive data or lead to <code>CSRF</code> which is an attack that allows attackers to trick
15+
users into performing unwanted operations in websites they're authenticated to.
16+
17+
</p>
18+
19+
</overview>
20+
21+
<recommendation>
22+
<p>
23+
24+
When the <code>origin</code> is set to <code>true</code>, it signifies that the server
25+
is accepting requests from <code>any</code> origin, potentially exposing the system to
26+
CSRF attacks. This can be fixed using <code>false</code> as origin value or using a whitelist.
27+
28+
</p>
29+
<p>
30+
31+
On the other hand, if the <code>origin</code> is
32+
set to <code>null</code>, it can be exploited by an attacker to deceive a user into making
33+
requests from a <code>null</code> origin form, often hosted within a sandboxed iframe.
34+
35+
</p>
36+
37+
<p>
38+
39+
If the <code>origin</code> value is user controlled, make sure that the data
40+
is properly sanitized.
41+
42+
</p>
43+
</recommendation>
44+
45+
<example>
46+
<p>
47+
48+
In the example below, the <code>server_1</code> accepts requests from any origin
49+
since the value of <code>origin</code> is set to <code>true</code>.
50+
And <code>server_2</code>'s origin is user-controlled.
51+
52+
</p>
53+
54+
<sample src="examples/CorsPermissiveConfigurationBad.js"/>
55+
56+
<p>
57+
58+
In the example below, the <code>server_1</code> CORS is restrictive so it's not
59+
vulnerable to CSRF attacks. And <code>server_2</code>'s is using properly sanitized
60+
user-controlled data.
61+
62+
</p>
63+
64+
<sample src="examples/CorsPermissiveConfigurationGood.js"/>
65+
</example>
66+
67+
<references>
68+
<li>Mozilla Developer Network: <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin">CORS, Access-Control-Allow-Origin</a>.</li>
69+
<li>W3C: <a href="https://w3c.github.io/webappsec-cors-for-developers/#resources">CORS for developers, Advice for Resource Owners</a></li>
70+
</references>
71+
</qhelp>

‎javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfiguration.ql

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* @name overly CORS configuration
3+
* @description Misconfiguration of CORS HTTP headers allows CSRF attacks.
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @security-severity 7.5
7+
* @precision high
8+
* @id js/cors-misconfiguration
9+
* @tags security
10+
* external/cwe/cwe-942
11+
*/
12+
13+
import javascript
14+
import CorsPermissiveConfigurationQuery
15+
import DataFlow::PathGraph
16+
17+
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
18+
where cfg.hasFlowPath(source, sink)
19+
select sink.getNode(), source, sink, "CORS Origin misconfiguration due to a $@.", source.getNode(),
20+
"too permissive or user controlled value"

‎javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationCustomizations.qll

+95
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/**
2+
* Provides default sources, sinks and sanitizers for reasoning about
3+
* overly permissive CORS configurations, as well as
4+
* extension points for adding your own.
5+
*/
6+
7+
import javascript
8+
import Cors::Cors
9+
import Apollo::Apollo
10+
11+
/** Module containing sources, sinks, and sanitizers for overly permissive CORS configurations. */
12+
module CorsPermissiveConfiguration {
13+
/**
14+
* A data flow source for permissive CORS configuration.
15+
*/
16+
abstract class Source extends DataFlow::Node { }
17+
18+
/**
19+
* A data flow sink for permissive CORS configuration.
20+
*/
21+
abstract class Sink extends DataFlow::Node { }
22+
23+
/**
24+
* A sanitizer for permissive CORS configuration.
25+
*/
26+
abstract class Sanitizer extends DataFlow::Node { }
27+
28+
/** A source of remote user input, considered as a flow source for CORS misconfiguration. */
29+
class RemoteFlowSourceAsSource extends Source instanceof RemoteFlowSource {
30+
RemoteFlowSourceAsSource() { not this instanceof ClientSideRemoteFlowSource }
31+
}
32+
33+
/** A flow label representing `true` and `null` values. */
34+
abstract class TrueAndNull extends DataFlow::FlowLabel {
35+
TrueAndNull() { this = "TrueAndNull" }
36+
}
37+
38+
TrueAndNull truenullLabel() { any() }
39+
40+
/** A flow label representing `*` value. */
41+
abstract class Wildcard extends DataFlow::FlowLabel {
42+
Wildcard() { this = "Wildcard" }
43+
}
44+
45+
Wildcard wildcardLabel() { any() }
46+
47+
/** An overly permissive value for `origin` (Apollo) */
48+
class TrueNullValue extends Source {
49+
TrueNullValue() { this.mayHaveBooleanValue(true) or this.asExpr() instanceof NullLiteral }
50+
}
51+
52+
/** An overly permissive value for `origin` (Express) */
53+
class WildcardValue extends Source {
54+
WildcardValue() { this.mayHaveStringValue("*") }
55+
}
56+
57+
/**
58+
* The value of cors origin when initializing the application.
59+
*/
60+
class CorsApolloServer extends Sink, DataFlow::ValueNode {
61+
CorsApolloServer() {
62+
exists(ApolloServer agql |
63+
this =
64+
agql.getOptionArgument(0, "cors").getALocalSource().getAPropertyWrite("origin").getRhs()
65+
)
66+
}
67+
}
68+
69+
/**
70+
* The value of cors origin when initializing the application.
71+
*/
72+
class ExpressCors extends Sink, DataFlow::ValueNode {
73+
ExpressCors() {
74+
exists(CorsConfiguration config | this = config.getCorsConfiguration().getOrigin())
75+
}
76+
}
77+
78+
/**
79+
* An express route setup configured with the `cors` package.
80+
*/
81+
class CorsConfiguration extends DataFlow::MethodCallNode {
82+
Cors corsConfig;
83+
84+
CorsConfiguration() {
85+
exists(Express::RouteSetup setup | this = setup |
86+
if setup.isUseCall()
87+
then corsConfig = setup.getArgument(0)
88+
else corsConfig = setup.getArgument(any(int i | i > 0))
89+
)
90+
}
91+
92+
/** Gets the expression that configures `cors` on this route setup. */
93+
Cors getCorsConfiguration() { result = corsConfig }
94+
}
95+
}

‎javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationQuery.qll

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/**
2+
* Provides a dataflow taint tracking configuration for reasoning
3+
* about overly permissive CORS configurations.
4+
*
5+
* Note, for performance reasons: only import this file if
6+
* `CorsPermissiveConfiguration::Configuration` is needed,
7+
* otherwise `CorsPermissiveConfigurationCustomizations` should
8+
* be imported instead.
9+
*/
10+
11+
import javascript
12+
import CorsPermissiveConfigurationCustomizations::CorsPermissiveConfiguration
13+
14+
/**
15+
* A data flow configuration for overly permissive CORS configuration.
16+
*/
17+
class Configuration extends TaintTracking::Configuration {
18+
Configuration() { this = "CorsPermissiveConfiguration" }
19+
20+
override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) {
21+
source instanceof TrueNullValue and label = truenullLabel()
22+
or
23+
source instanceof WildcardValue and label = wildcardLabel()
24+
or
25+
source instanceof RemoteFlowSource and label = DataFlow::FlowLabel::taint()
26+
}
27+
28+
override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
29+
sink instanceof CorsApolloServer and label = [DataFlow::FlowLabel::taint(), truenullLabel()]
30+
or
31+
sink instanceof ExpressCors and label = [DataFlow::FlowLabel::taint(), wildcardLabel()]
32+
}
33+
34+
override predicate isSanitizer(DataFlow::Node node) {
35+
super.isSanitizer(node) or
36+
node instanceof Sanitizer
37+
}
38+
}
39+
40+
private class WildcardActivated extends DataFlow::FlowLabel, Wildcard {
41+
WildcardActivated() { this = this }
42+
}
43+
44+
private class TrueAndNullActivated extends DataFlow::FlowLabel, TrueAndNull {
45+
TrueAndNullActivated() { this = this }
46+
}

‎javascript/ql/src/experimental/Security/CWE-942/examples/CorsPermissiveConfigurationBad.js

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { ApolloServer } from 'apollo-server';
2+
var https = require('https'),
3+
url = require('url');
4+
5+
var server = https.createServer(function () { });
6+
7+
server.on('request', function (req, res) {
8+
// BAD: origin is too permissive
9+
const server_1 = new ApolloServer({
10+
cors: { origin: true }
11+
});
12+
13+
let user_origin = url.parse(req.url, true).query.origin;
14+
// BAD: CORS is controlled by user
15+
const server_2 = new ApolloServer({
16+
cors: { origin: user_origin }
17+
});
18+
});

‎javascript/ql/src/experimental/Security/CWE-942/examples/CorsPermissiveConfigurationGood.js

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { ApolloServer } from 'apollo-server';
2+
var https = require('https'),
3+
url = require('url');
4+
5+
var server = https.createServer(function () { });
6+
7+
server.on('request', function (req, res) {
8+
// GOOD: origin is restrictive
9+
const server_1 = new ApolloServer({
10+
cors: { origin: false }
11+
});
12+
13+
let user_origin = url.parse(req.url, true).query.origin;
14+
// GOOD: user data is properly sanitized
15+
const server_2 = new ApolloServer({
16+
cors: { origin: (user_origin === "https://allowed1.com" || user_origin === "https://allowed2.com") ? user_origin : false }
17+
});
18+
});

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

+50
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
nodes
2+
| apollo-test.js:8:9:8:59 | user_origin |
3+
| apollo-test.js:8:23:8:46 | url.par ... , true) |
4+
| apollo-test.js:8:23:8:52 | url.par ... ).query |
5+
| apollo-test.js:8:23:8:59 | url.par ... .origin |
6+
| apollo-test.js:8:33:8:39 | req.url |
7+
| apollo-test.js:8:33:8:39 | req.url |
8+
| apollo-test.js:11:25:11:28 | true |
9+
| apollo-test.js:11:25:11:28 | true |
10+
| apollo-test.js:11:25:11:28 | true |
11+
| apollo-test.js:21:25:21:28 | null |
12+
| apollo-test.js:21:25:21:28 | null |
13+
| apollo-test.js:21:25:21:28 | null |
14+
| apollo-test.js:26:25:26:35 | user_origin |
15+
| apollo-test.js:26:25:26:35 | user_origin |
16+
| express-test.js:10:9:10:59 | user_origin |
17+
| express-test.js:10:23:10:46 | url.par ... , true) |
18+
| express-test.js:10:23:10:52 | url.par ... ).query |
19+
| express-test.js:10:23:10:59 | url.par ... .origin |
20+
| express-test.js:10:33:10:39 | req.url |
21+
| express-test.js:10:33:10:39 | req.url |
22+
| express-test.js:26:17:26:19 | '*' |
23+
| express-test.js:26:17:26:19 | '*' |
24+
| express-test.js:26:17:26:19 | '*' |
25+
| express-test.js:33:17:33:27 | user_origin |
26+
| express-test.js:33:17:33:27 | user_origin |
27+
edges
28+
| apollo-test.js:8:9:8:59 | user_origin | apollo-test.js:26:25:26:35 | user_origin |
29+
| apollo-test.js:8:9:8:59 | user_origin | apollo-test.js:26:25:26:35 | user_origin |
30+
| apollo-test.js:8:23:8:46 | url.par ... , true) | apollo-test.js:8:23:8:52 | url.par ... ).query |
31+
| apollo-test.js:8:23:8:52 | url.par ... ).query | apollo-test.js:8:23:8:59 | url.par ... .origin |
32+
| apollo-test.js:8:23:8:59 | url.par ... .origin | apollo-test.js:8:9:8:59 | user_origin |
33+
| apollo-test.js:8:33:8:39 | req.url | apollo-test.js:8:23:8:46 | url.par ... , true) |
34+
| apollo-test.js:8:33:8:39 | req.url | apollo-test.js:8:23:8:46 | url.par ... , true) |
35+
| apollo-test.js:11:25:11:28 | true | apollo-test.js:11:25:11:28 | true |
36+
| apollo-test.js:21:25:21:28 | null | apollo-test.js:21:25:21:28 | null |
37+
| express-test.js:10:9:10:59 | user_origin | express-test.js:33:17:33:27 | user_origin |
38+
| express-test.js:10:9:10:59 | user_origin | express-test.js:33:17:33:27 | user_origin |
39+
| express-test.js:10:23:10:46 | url.par ... , true) | express-test.js:10:23:10:52 | url.par ... ).query |
40+
| express-test.js:10:23:10:52 | url.par ... ).query | express-test.js:10:23:10:59 | url.par ... .origin |
41+
| express-test.js:10:23:10:59 | url.par ... .origin | express-test.js:10:9:10:59 | user_origin |
42+
| express-test.js:10:33:10:39 | req.url | express-test.js:10:23:10:46 | url.par ... , true) |
43+
| express-test.js:10:33:10:39 | req.url | express-test.js:10:23:10:46 | url.par ... , true) |
44+
| express-test.js:26:17:26:19 | '*' | express-test.js:26:17:26:19 | '*' |
45+
#select
46+
| 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 |
47+
| 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 |
48+
| 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 |
49+
| 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 |
50+
| 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 |

‎javascript/ql/test/experimental/Security/CWE-942/CorsPermissiveConfiguration.qlref

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
./experimental/Security/CWE-942/CorsPermissiveConfiguration.ql

0 commit comments

Comments
 (0)