Skip to content
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

Merged
merged 31 commits into from
Jun 28, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
e171123
Add initial query for CWE-942
maikypedia Sep 29, 2023
142ab01
Remove comment line
maikypedia Sep 29, 2023
816eebb
Add `.qhelp` and apply some review changes
maikypedia Oct 2, 2023
ed06628
Add documentation string for `CorsPermissiveConfiguration`
maikypedia Oct 6, 2023
c0e6d7c
Merge branch 'github:main' into maikypedia/javascript-cors
maikypedia Oct 11, 2023
07ad596
Add coverage for `express`
maikypedia Oct 16, 2023
acac534
Forgot `.js`
maikypedia Oct 16, 2023
d661f7f
Add Flow Labels
maikypedia Nov 22, 2023
413c111
Move to `/experimental`
maikypedia Nov 23, 2023
abd53e9
Fix minor issues
maikypedia Nov 23, 2023
4ef4c92
Move Customizations and Query
maikypedia Nov 23, 2023
aa24ce5
Apply suggestions from code review
maikypedia Nov 27, 2023
bb6ef72
`getArgument` returns `Cors::Cors`
maikypedia Nov 27, 2023
f623db4
Change qldoc
maikypedia Nov 27, 2023
3bcb411
Using `Express::RouteSetup`
maikypedia Nov 27, 2023
6a3cdc9
Add `change-node`
maikypedia Nov 27, 2023
e6c7fc0
Fixes CI
maikypedia Nov 29, 2023
83cbbd7
Apply docstring changes
maikypedia Dec 5, 2023
87cac2a
Express Argument has to be Cors
maikypedia Dec 7, 2023
4f68f60
Apply review
maikypedia Dec 18, 2023
191766a
Use `config.getCorsConfiguration().getOrigin())`
maikypedia Dec 18, 2023
7662b2b
format
maikypedia Dec 19, 2023
78e7793
Move to experimental
maikypedia Jan 9, 2024
699d8d4
x
maikypedia Mar 7, 2024
c1fd7a6
autoformat
erik-krogh Mar 12, 2024
f2d6640
fix ambiguous import. It could refer both to a module or a file
erik-krogh Mar 12, 2024
cfd7c7a
move change-note to `javascript/ql/src/change-notes`
maikypedia May 27, 2024
e96c3a3
Move `Apollo` to experimental
maikypedia May 27, 2024
4be5cf4
Update javascript/ql/src/experimental/Security/CWE-942/CorsPermissive…
maikypedia Jun 12, 2024
8ba7ac6
Update javascript/ql/src/experimental/Security/CWE-942/CorsPermissive…
maikypedia Jun 12, 2024
d0cf2a9
Merge branch 'main' into maikypedia/javascript-cors
maikypedia Jun 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, this change-note should probably be in the javascript/ql/src/change-notes folder instead.

* Added a new experimental query, `js/cors-misconfiguration`, which detects misconfigured CORS HTTP headers in the `cors` and `apollo` libraries.
1 change: 1 addition & 0 deletions javascript/ql/lib/javascript.qll
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ import semmle.javascript.frameworks.ActionsLib
import semmle.javascript.frameworks.Angular2
import semmle.javascript.frameworks.AngularJS
import semmle.javascript.frameworks.Anser
import semmle.javascript.frameworks.Apollo
import semmle.javascript.frameworks.AsyncPackage
import semmle.javascript.frameworks.AWS
import semmle.javascript.frameworks.Azure
Expand Down
36 changes: 36 additions & 0 deletions javascript/ql/lib/semmle/javascript/frameworks/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
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done 👍


/** 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 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/lib/semmle/javascript/frameworks/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") }
}
}
26 changes: 26 additions & 0 deletions javascript/ql/lib/semmle/javascript/frameworks/Express.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -1071,4 +1072,29 @@ module Express {

override predicate definitelyResumesDispatch() { none() }
}

/**
* An express route setup configured with the `cors` package.
*/
class CorsConfiguration extends DataFlow::MethodCallNode {
CorsConfiguration() {
exists(Express::RouteSetup setup | this = setup |
setup.isUseCall() and setup.getArgument(0) instanceof Cors::Cors
or
not setup.isUseCall() and setup.getAnArgument() instanceof Cors::Cors
)
}

/** Gets the cors argument */
Cors::Cors getArgument() { result = this.getArgument(0) }

/** Gets the options used to configure `cors`. */
DataFlow::Node getCorsArgument() { result = this.getArgument().getOptionsArgument() }

/** Holds if cors is using its default configuration. */
predicate isDefault() { this.getArgument().isDefault() }

/** Gets the `origin` option that the call to `cors` is configured with. */
DataFlow::Node getOrigin() { result = this.getArgument().getOrigin() }
}
maikypedia marked this conversation as resolved.
Show resolved Hide resolved
}
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,73 @@
/**
* 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 {
/**
* 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(Apollo::ApolloServer agql |
maikypedia marked this conversation as resolved.
Show resolved Hide resolved
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()) }
}
maikypedia marked this conversation as resolved.
Show resolved Hide resolved
}
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 }
});
});
Loading
Loading