Skip to content

Commit

Permalink
Merge pull request #14342 from maikypedia/maikypedia/javascript-cors
Browse files Browse the repository at this point in the history
JS: Add Permissive CORS query (CWE-942)
  • Loading branch information
erik-krogh authored Jun 28, 2024
2 parents 72caadb + d0cf2a9 commit fd3089e
Show file tree
Hide file tree
Showing 13 changed files with 447 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Added a new experimental query, `js/cors-misconfiguration`, which detects misconfigured CORS HTTP headers in the `cors` and `apollo` libraries.
36 changes: 36 additions & 0 deletions javascript/ql/src/experimental/Security/CWE-942/Apollo.qll
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 {
/** Get a reference to the `ApolloServer` class. */
private API::Node apollo() {
result =
API::moduleImport([
"@apollo/server", "@apollo/apollo-server-express", "@apollo/apollo-server-core",
"apollo-server", "apollo-server-express"
]).getMember("ApolloServer")
}

/** Gets a reference to the `gql` function that parses GraphQL strings. */
private API::Node gql() {
result =
API::moduleImport([
"@apollo/server", "@apollo/apollo-server-express", "@apollo/apollo-server-core",
"apollo-server", "apollo-server-express"
]).getMember("gql")
}

/** An instantiation of an `ApolloServer`. */
class ApolloServer extends API::NewNode {
ApolloServer() { this = apollo().getAnInstantiation() }
}

/** A string that is interpreted as a GraphQL query by a `apollo` package. */
private class ApolloGraphQLString extends GraphQL::GraphQLString {
ApolloGraphQLString() { this = gql().getACall().getArgument(0) }
}
}
24 changes: 24 additions & 0 deletions javascript/ql/src/experimental/Security/CWE-942/Cors.qll
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 the [cors](https://npmjs.com/package/cors) library. */
module Cors {
/**
* An expression that creates a new CORS configuration.
*/
class Cors extends DataFlow::CallNode {
Cors() { this = DataFlow::moduleImport("cors").getAnInvocation() }

/** Get the options used to configure Cors */
DataFlow::Node getOptionsArgument() { result = this.getArgument(0) }

/** Holds if cors is using default configuration */
predicate isDefault() { this.getNumArgument() = 0 }

/** Gets the value of the `origin` option used to configure this Cors instance. */
DataFlow::Node getOrigin() { result = this.getOptionArgument(0, "origin") }
}
}
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 CorsPermissiveConfigurationQuery
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,95 @@
/**
* Provides default sources, sinks and sanitizers for reasoning about
* overly permissive CORS configurations, as well as
* extension points for adding your own.
*/

import javascript
import Cors::Cors
import Apollo::Apollo

/** Module containing sources, sinks, and sanitizers for overly permissive CORS configurations. */
module CorsPermissiveConfiguration {
/**
* 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 }
}

/** A flow label representing `true` and `null` values. */
abstract class TrueAndNull extends DataFlow::FlowLabel {
TrueAndNull() { this = "TrueAndNull" }
}

TrueAndNull truenullLabel() { any() }

/** A flow label representing `*` value. */
abstract class Wildcard extends DataFlow::FlowLabel {
Wildcard() { this = "Wildcard" }
}

Wildcard wildcardLabel() { any() }

/** An overly permissive value for `origin` (Apollo) */
class TrueNullValue extends Source {
TrueNullValue() { this.mayHaveBooleanValue(true) or this.asExpr() instanceof NullLiteral }
}

/** An overly permissive value for `origin` (Express) */
class WildcardValue extends Source {
WildcardValue() { this.mayHaveStringValue("*") }
}

/**
* The value of cors origin when initializing the application.
*/
class CorsApolloServer extends Sink, DataFlow::ValueNode {
CorsApolloServer() {
exists(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(CorsConfiguration config | this = config.getCorsConfiguration().getOrigin())
}
}

/**
* An express route setup configured with the `cors` package.
*/
class CorsConfiguration extends DataFlow::MethodCallNode {
Cors corsConfig;

CorsConfiguration() {
exists(Express::RouteSetup setup | this = setup |
if setup.isUseCall()
then corsConfig = setup.getArgument(0)
else corsConfig = setup.getArgument(any(int i | i > 0))
)
}

/** Gets the expression that configures `cors` on this route setup. */
Cors getCorsConfiguration() { result = corsConfig }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/**
* 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, DataFlow::FlowLabel label) {
source instanceof TrueNullValue and label = truenullLabel()
or
source instanceof WildcardValue and label = wildcardLabel()
or
source instanceof RemoteFlowSource and label = DataFlow::FlowLabel::taint()
}

override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
sink instanceof CorsApolloServer and label = [DataFlow::FlowLabel::taint(), truenullLabel()]
or
sink instanceof ExpressCors and label = [DataFlow::FlowLabel::taint(), wildcardLabel()]
}

override predicate isSanitizer(DataFlow::Node node) {
super.isSanitizer(node) or
node instanceof Sanitizer
}
}

private class WildcardActivated extends DataFlow::FlowLabel, Wildcard {
WildcardActivated() { this = this }
}

private class TrueAndNullActivated extends DataFlow::FlowLabel, TrueAndNull {
TrueAndNullActivated() { this = this }
}
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,50 @@
nodes
| apollo-test.js:8:9:8:59 | user_origin |
| apollo-test.js:8:23:8:46 | url.par ... , true) |
| apollo-test.js:8:23:8:52 | url.par ... ).query |
| apollo-test.js:8:23:8:59 | url.par ... .origin |
| apollo-test.js:8:33:8:39 | req.url |
| apollo-test.js:8:33:8:39 | req.url |
| apollo-test.js:11:25:11:28 | true |
| apollo-test.js:11:25:11:28 | true |
| apollo-test.js:11:25:11:28 | true |
| apollo-test.js:21:25:21:28 | null |
| apollo-test.js:21:25:21:28 | null |
| apollo-test.js:21:25:21:28 | null |
| apollo-test.js:26:25:26:35 | user_origin |
| apollo-test.js:26:25:26:35 | user_origin |
| express-test.js:10:9:10:59 | user_origin |
| express-test.js:10:23:10:46 | url.par ... , true) |
| express-test.js:10:23:10:52 | url.par ... ).query |
| express-test.js:10:23:10:59 | url.par ... .origin |
| express-test.js:10:33:10:39 | req.url |
| express-test.js:10:33:10:39 | req.url |
| express-test.js:26:17:26:19 | '*' |
| express-test.js:26:17:26:19 | '*' |
| express-test.js:26:17:26:19 | '*' |
| express-test.js:33:17:33:27 | user_origin |
| express-test.js:33:17:33:27 | user_origin |
edges
| apollo-test.js:8:9:8:59 | user_origin | apollo-test.js:26:25:26:35 | user_origin |
| apollo-test.js:8:9:8:59 | user_origin | apollo-test.js:26:25:26:35 | user_origin |
| apollo-test.js:8:23:8:46 | url.par ... , true) | apollo-test.js:8:23:8:52 | url.par ... ).query |
| apollo-test.js:8:23:8:52 | url.par ... ).query | apollo-test.js:8:23:8:59 | url.par ... .origin |
| apollo-test.js:8:23:8:59 | url.par ... .origin | apollo-test.js:8:9:8:59 | user_origin |
| apollo-test.js:8:33:8:39 | req.url | apollo-test.js:8:23:8:46 | url.par ... , true) |
| apollo-test.js:8:33:8:39 | req.url | apollo-test.js:8:23:8:46 | url.par ... , true) |
| apollo-test.js:11:25:11:28 | true | apollo-test.js:11:25:11:28 | true |
| apollo-test.js:21:25:21:28 | null | apollo-test.js:21:25:21:28 | null |
| express-test.js:10:9:10:59 | user_origin | express-test.js:33:17:33:27 | user_origin |
| express-test.js:10:9:10:59 | user_origin | express-test.js:33:17:33:27 | user_origin |
| express-test.js:10:23:10:46 | url.par ... , true) | express-test.js:10:23:10:52 | url.par ... ).query |
| express-test.js:10:23:10:52 | url.par ... ).query | express-test.js:10:23:10:59 | url.par ... .origin |
| express-test.js:10:23:10:59 | url.par ... .origin | express-test.js:10:9:10:59 | user_origin |
| express-test.js:10:33:10:39 | req.url | express-test.js:10:23:10:46 | url.par ... , true) |
| express-test.js:10:33:10:39 | req.url | express-test.js:10:23:10:46 | url.par ... , true) |
| express-test.js:26:17:26:19 | '*' | express-test.js:26:17:26:19 | '*' |
#select
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
./experimental/Security/CWE-942/CorsPermissiveConfiguration.ql
Loading

0 comments on commit fd3089e

Please sign in to comment.