-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JS: Add Permissive CORS query (CWE-942) #14342
Changes from 7 commits
e171123
142ab01
816eebb
ed06628
c0e6d7c
07ad596
acac534
d661f7f
413c111
abd53e9
4ef4c92
aa24ce5
bb6ef72
f623db4
3bcb411
6a3cdc9
e6c7fc0
83cbbd7
87cac2a
4f68f60
191766a
7662b2b
78e7793
699d8d4
c1fd7a6
f2d6640
cfd7c7a
e96c3a3
4be5cf4
8ba7ac6
d0cf2a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
/** | ||
* Provides classes for working with Apollo GraphQL connectors. | ||
*/ | ||
|
||
import javascript | ||
|
||
/** Provides classes modeling the apollo packages [@apollo/server](https://npmjs.com/package/@apollo/server`) */ | ||
module Apollo { | ||
maikypedia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/** Get an instanceof of `Apollo` */ | ||
maikypedia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private API::Node apollo() { | ||
result = | ||
API::moduleImport([ | ||
"@apollo/server", "@apollo/apollo-server-express", "@apollo/apollo-server-core", | ||
"apollo-server", "apollo-server-express" | ||
]).getMember("ApolloServer") | ||
} | ||
|
||
/** Get an instanceof of the `gql` function that parses GraphQL strings. */ | ||
maikypedia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private API::Node gql() { | ||
result = | ||
API::moduleImport([ | ||
"@apollo/server", "@apollo/apollo-server-express", "@apollo/apollo-server-core", | ||
"apollo-server", "apollo-server-express" | ||
]).getMember("gql") | ||
} | ||
|
||
/** A string that is interpreted as a GraphQL query by a `graphql` package. */ | ||
maikypedia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
class ApolloServer extends API::NewNode { | ||
ApolloServer() { this = apollo().getAnInstantiation() } | ||
} | ||
|
||
/** A string that is interpreted as a GraphQL query by a `apollo` package. */ | ||
class ApolloGraphQLString extends GraphQL::GraphQLString { | ||
maikypedia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ApolloGraphQLString() { this = gql().getACall().getArgument(0) } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
/** | ||
* Provides classes for working with Cors connectors. | ||
*/ | ||
|
||
import javascript | ||
|
||
/** Provides classes modeling [cors package](https://npmjs.com/package/cors) */ | ||
maikypedia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
module Cors { | ||
class Cors extends DataFlow::CallNode { | ||
/** Get an instanceof of `cors` */ | ||
Cors() { this = DataFlow::moduleImport("cors").getAnInvocation() } | ||
maikypedia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/** Get Cors configuration */ | ||
maikypedia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
DataFlow::Node getCorsArgument() { result = this.getArgument(0) } | ||
maikypedia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/** Holds if cors is using default configuration */ | ||
predicate isDefault() { this.getNumArgument() = 0 } | ||
|
||
/** The value of origin */ | ||
|
||
DataFlow::Node getOrigin() { | ||
result = this.getCorsArgument().getALocalSource().getAPropertyWrite("origin").getRhs() | ||
maikypedia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
import javascript | ||
import semmle.javascript.frameworks.HTTP | ||
import semmle.javascript.frameworks.ExpressModules | ||
import semmle.javascript.frameworks.Cors | ||
private import semmle.javascript.dataflow.InferredTypes | ||
private import semmle.javascript.frameworks.ConnectExpressShared::ConnectExpressShared | ||
|
||
|
@@ -1071,4 +1072,23 @@ | |
|
||
override predicate definitelyResumesDispatch() { none() } | ||
} | ||
|
||
class CorsConfiguration extends DataFlow::MethodCallNode { | ||
/** Get an `app.use` with a cors object as argument */ | ||
CorsConfiguration() { | ||
this = appCreation().getAMethodCall("use") and this.getArgument(0) instanceof Cors::Cors | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have the Additionally. The |
||
} | ||
|
||
/** Get Cors */ | ||
private Cors::Cors cors() { result = this.getArgument(0).(Cors::Cors) } | ||
|
||
|
||
/** Get Cors configuration */ | ||
maikypedia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
DataFlow::Node getCorsArgument() { result = cors().getCorsArgument() } | ||
|
||
|
||
/** Holds if cors is using default configuration */ | ||
maikypedia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
predicate isDefault() { cors().isDefault() } | ||
|
||
|
||
/** Get Cors origin value */ | ||
maikypedia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
DataFlow::Node getOrigin() { result = cors().getOrigin() } | ||
|
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
/** | ||
* Provides default sources, sinks and sanitizers for reasoning about | ||
* overly permissive CORS configurations, as well as | ||
* extension points for adding your own. | ||
*/ | ||
|
||
import javascript | ||
|
||
/** Module containing sources, sinks, and sanitizers for overly permissive CORS configurations. */ | ||
module CorsPermissiveConfiguration { | ||
maikypedia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/** | ||
* A data flow source for permissive CORS configuration. | ||
*/ | ||
abstract class Source extends DataFlow::Node { } | ||
|
||
/** | ||
* A data flow sink for permissive CORS configuration. | ||
*/ | ||
abstract class Sink extends DataFlow::Node { } | ||
|
||
/** | ||
* A sanitizer for permissive CORS configuration. | ||
*/ | ||
abstract class Sanitizer extends DataFlow::Node { } | ||
|
||
/** A source of remote user input, considered as a flow source for CORS misconfiguration. */ | ||
class RemoteFlowSourceAsSource extends Source instanceof RemoteFlowSource { | ||
RemoteFlowSourceAsSource() { not this instanceof ClientSideRemoteFlowSource } | ||
} | ||
|
||
/** An overfly permissive value for `origin` */ | ||
class BadValues extends Source { | ||
BadValues() { this.mayHaveBooleanValue(true) or this.asExpr() instanceof NullLiteral } | ||
} | ||
|
||
/** | ||
* The value of cors origin when initializing the application. | ||
*/ | ||
class CorsApolloServer extends Sink, DataFlow::ValueNode { | ||
CorsApolloServer() { | ||
exists(Apollo::ApolloServer agql | | ||
this = | ||
agql.getOptionArgument(0, "cors").getALocalSource().getAPropertyWrite("origin").getRhs() | ||
) | ||
} | ||
} | ||
|
||
/** | ||
* The value of cors origin when initializing the application. | ||
*/ | ||
class ExpressCors extends Sink, DataFlow::ValueNode { | ||
ExpressCors() { exists(Express::CorsConfiguration config | this = config.getOrigin()) } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
/** | ||
* Provides a dataflow taint tracking configuration for reasoning | ||
* about overly permissive CORS configurations. | ||
* | ||
* Note, for performance reasons: only import this file if | ||
* `CorsPermissiveConfiguration::Configuration` is needed, | ||
* otherwise `CorsPermissiveConfigurationCustomizations` should | ||
* be imported instead. | ||
*/ | ||
|
||
import javascript | ||
import CorsPermissiveConfigurationCustomizations::CorsPermissiveConfiguration | ||
|
||
/** | ||
* A data flow configuration for overly permissive CORS configuration. | ||
*/ | ||
class Configuration extends TaintTracking::Configuration { | ||
Configuration() { this = "CorsPermissiveConfiguration" } | ||
|
||
override predicate isSource(DataFlow::Node source) { source instanceof Source } | ||
|
||
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink } | ||
|
||
override predicate isSanitizer(DataFlow::Node node) { | ||
super.isSanitizer(node) or | ||
node instanceof Sanitizer | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
<!DOCTYPE qhelp PUBLIC | ||
"-//Semmle//qhelp//EN" | ||
"qhelp.dtd"> | ||
<qhelp> | ||
|
||
<overview> | ||
<p> | ||
|
||
A server can use <code>CORS</code> (Cross-Origin Resource Sharing) to relax the | ||
restrictions imposed by the <code>SOP</code> (Same-Origin Policy), allowing controlled, secure | ||
cross-origin requests when necessary. | ||
|
||
A server with an overly permissive <code>CORS</code> configuration may inadvertently | ||
expose sensitive data or lead to <code>CSRF</code> which is an attack that allows attackers to trick | ||
users into performing unwanted operations in websites they're authenticated to. | ||
|
||
</p> | ||
|
||
</overview> | ||
|
||
<recommendation> | ||
<p> | ||
|
||
When the <code>origin</code> is set to <code>true</code>, it signifies that the server | ||
is accepting requests from <code>any</code> origin, potentially exposing the system to | ||
CSRF attacks. This can be fixed using <code>false</code> as origin value or using a whitelist. | ||
|
||
</p> | ||
<p> | ||
|
||
On the other hand, if the <code>origin</code> is | ||
set to <code>null</code>, it can be exploited by an attacker to deceive a user into making | ||
requests from a <code>null</code> origin form, often hosted within a sandboxed iframe. | ||
|
||
</p> | ||
|
||
<p> | ||
|
||
If the <code>origin</code> value is user controlled, make sure that the data | ||
is properly sanitized. | ||
|
||
</p> | ||
</recommendation> | ||
|
||
<example> | ||
<p> | ||
|
||
In the example below, the <code>server_1</code> accepts requests from any origin | ||
since the value of <code>origin</code> is set to <code>true</code>. | ||
And <code>server_2</code>'s origin is user-controlled. | ||
|
||
</p> | ||
|
||
<sample src="examples/CorsPermissiveConfigurationBad.js"/> | ||
|
||
<p> | ||
|
||
In the example below, the <code>server_1</code> CORS is restrictive so it's not | ||
vulnerable to CSRF attacks. And <code>server_2</code>'s is using properly sanitized | ||
user-controlled data. | ||
|
||
</p> | ||
|
||
<sample src="examples/CorsPermissiveConfigurationGood.js"/> | ||
</example> | ||
|
||
<references> | ||
<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> | ||
<li>W3C: <a href="https://w3c.github.io/webappsec-cors-for-developers/#resources">CORS for developers, Advice for Resource Owners</a></li> | ||
</references> | ||
</qhelp> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
/** | ||
* @name overly CORS configuration | ||
* @description Misconfiguration of CORS HTTP headers allows CSRF attacks. | ||
* @kind path-problem | ||
* @problem.severity error | ||
* @security-severity 7.5 | ||
* @precision high | ||
* @id js/cors-misconfiguration | ||
* @tags security | ||
* external/cwe/cwe-942 | ||
*/ | ||
|
||
import javascript | ||
import semmle.javascript.security.dataflow.CorsPermissiveConfigurationQuery | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This import doesn't work after you moved to experimental. I think you should just move the two |
||
import DataFlow::PathGraph | ||
|
||
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink | ||
where cfg.hasFlowPath(source, sink) | ||
select sink.getNode(), source, sink, "CORS Origin misconfiguration due to a $@.", source.getNode(), | ||
"too permissive or user controlled value" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import { ApolloServer } from 'apollo-server'; | ||
var https = require('https'), | ||
url = require('url'); | ||
|
||
var server = https.createServer(function () { }); | ||
|
||
server.on('request', function (req, res) { | ||
// BAD: origin is too permissive | ||
const server_1 = new ApolloServer({ | ||
cors: { origin: true } | ||
}); | ||
|
||
let user_origin = url.parse(req.url, true).query.origin; | ||
// BAD: CORS is controlled by user | ||
const server_2 = new ApolloServer({ | ||
cors: { origin: user_origin } | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import { ApolloServer } from 'apollo-server'; | ||
var https = require('https'), | ||
url = require('url'); | ||
|
||
var server = https.createServer(function () { }); | ||
|
||
server.on('request', function (req, res) { | ||
// GOOD: origin is restrictive | ||
const server_1 = new ApolloServer({ | ||
cors: { origin: false } | ||
}); | ||
|
||
let user_origin = url.parse(req.url, true).query.origin; | ||
// GOOD: user data is properly sanitized | ||
const server_2 = new ApolloServer({ | ||
cors: { origin: (user_origin === "https://allowed1.com" || user_origin === "https://allowed2.com") ? user_origin : false } | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
nodes | ||
| tst.js:8:9:8:59 | user_origin | | ||
| tst.js:8:23:8:46 | url.par ... , true) | | ||
| tst.js:8:23:8:52 | url.par ... ).query | | ||
| tst.js:8:23:8:59 | url.par ... .origin | | ||
| tst.js:8:33:8:39 | req.url | | ||
| tst.js:8:33:8:39 | req.url | | ||
| tst.js:8:42:8:45 | true | | ||
| tst.js:8:42:8:45 | true | | ||
| tst.js:11:25:11:28 | true | | ||
| tst.js:11:25:11:28 | true | | ||
| tst.js:11:25:11:28 | true | | ||
| tst.js:21:25:21:28 | null | | ||
| tst.js:21:25:21:28 | null | | ||
| tst.js:21:25:21:28 | null | | ||
| tst.js:26:25:26:35 | user_origin | | ||
| tst.js:26:25:26:35 | user_origin | | ||
edges | ||
| tst.js:8:9:8:59 | user_origin | tst.js:26:25:26:35 | user_origin | | ||
| tst.js:8:9:8:59 | user_origin | tst.js:26:25:26:35 | user_origin | | ||
| tst.js:8:23:8:46 | url.par ... , true) | tst.js:8:23:8:52 | url.par ... ).query | | ||
| tst.js:8:23:8:52 | url.par ... ).query | tst.js:8:23:8:59 | url.par ... .origin | | ||
| tst.js:8:23:8:59 | url.par ... .origin | tst.js:8:9:8:59 | user_origin | | ||
| tst.js:8:33:8:39 | req.url | tst.js:8:23:8:46 | url.par ... , true) | | ||
| tst.js:8:33:8:39 | req.url | tst.js:8:23:8:46 | url.par ... , true) | | ||
| tst.js:8:42:8:45 | true | tst.js:8:23:8:46 | url.par ... , true) | | ||
| tst.js:8:42:8:45 | true | tst.js:8:23:8:46 | url.par ... , true) | | ||
| tst.js:11:25:11:28 | true | tst.js:11:25:11:28 | true | | ||
| tst.js:21:25:21:28 | null | tst.js:21:25:21:28 | null | | ||
#select | ||
| tst.js:11:25:11:28 | true | tst.js:11:25:11:28 | true | tst.js:11:25:11:28 | true | CORS Origin misconfiguration due to a $@. | tst.js:11:25:11:28 | true | too permissive or user controlled value | | ||
| tst.js:21:25:21:28 | null | tst.js:21:25:21:28 | null | tst.js:21:25:21:28 | null | CORS Origin misconfiguration due to a $@. | tst.js:21:25:21:28 | null | too permissive or user controlled value | | ||
| tst.js:26:25:26:35 | user_origin | tst.js:8:33:8:39 | req.url | tst.js:26:25:26:35 | user_origin | CORS Origin misconfiguration due to a $@. | tst.js:8:33:8:39 | req.url | too permissive or user controlled value | | ||
| tst.js:26:25:26:35 | user_origin | tst.js:8:42:8:45 | true | tst.js:26:25:26:35 | user_origin | CORS Origin misconfiguration due to a $@. | tst.js:8:42:8:45 | true | too permissive or user controlled value | |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
./experimental/Security/CWE-942/CorsPermissiveConfiguration.ql |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
import { ApolloServer } from 'apollo-server'; | ||
var https = require('https'), | ||
url = require('url'); | ||
|
||
var server = https.createServer(function () { }); | ||
|
||
server.on('request', function (req, res) { | ||
let user_origin = url.parse(req.url, true).query.origin; | ||
// BAD: CORS too permissive | ||
const server_1 = new ApolloServer({ | ||
cors: { origin: true } | ||
}); | ||
|
||
// GOOD: restrictive CORS | ||
const server_2 = new ApolloServer({ | ||
cors: false | ||
}); | ||
|
||
// BAD: CORS too permissive | ||
const server_3 = new ApolloServer({ | ||
cors: { origin: null } | ||
}); | ||
|
||
// BAD: CORS is controlled by user | ||
const server_4 = new ApolloServer({ | ||
cors: { origin: user_origin } | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is only used within your experimental query, so it should probably be moved to the same folder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done 👍