diff --git a/javascript/ql/src/change-notes/2023-11-27-cors-permissive-configuarion.md b/javascript/ql/src/change-notes/2023-11-27-cors-permissive-configuarion.md new file mode 100644 index 000000000000..877a54a9d8e8 --- /dev/null +++ b/javascript/ql/src/change-notes/2023-11-27-cors-permissive-configuarion.md @@ -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. \ No newline at end of file diff --git a/javascript/ql/src/experimental/Security/CWE-942/Apollo.qll b/javascript/ql/src/experimental/Security/CWE-942/Apollo.qll new file mode 100644 index 000000000000..983c0a8ac89c --- /dev/null +++ b/javascript/ql/src/experimental/Security/CWE-942/Apollo.qll @@ -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) } + } +} diff --git a/javascript/ql/src/experimental/Security/CWE-942/Cors.qll b/javascript/ql/src/experimental/Security/CWE-942/Cors.qll new file mode 100644 index 000000000000..cc190e6f4294 --- /dev/null +++ b/javascript/ql/src/experimental/Security/CWE-942/Cors.qll @@ -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") } + } +} diff --git a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfiguration.qhelp b/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfiguration.qhelp new file mode 100644 index 000000000000..fc79eee743bf --- /dev/null +++ b/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfiguration.qhelp @@ -0,0 +1,71 @@ + + + + +

+ + A server can use CORS (Cross-Origin Resource Sharing) to relax the + restrictions imposed by the SOP (Same-Origin Policy), allowing controlled, secure + cross-origin requests when necessary. + + A server with an overly permissive CORS configuration may inadvertently + expose sensitive data or lead to CSRF which is an attack that allows attackers to trick + users into performing unwanted operations in websites they're authenticated to. + +

+ +
+ + +

+ + When the origin is set to true, it signifies that the server + is accepting requests from any origin, potentially exposing the system to + CSRF attacks. This can be fixed using false as origin value or using a whitelist. + +

+

+ + On the other hand, if the origin is + set to null, it can be exploited by an attacker to deceive a user into making + requests from a null origin form, often hosted within a sandboxed iframe. + +

+ +

+ + If the origin value is user controlled, make sure that the data + is properly sanitized. + +

+
+ + +

+ + In the example below, the server_1 accepts requests from any origin + since the value of origin is set to true. + And server_2's origin is user-controlled. + +

+ + + +

+ + In the example below, the server_1 CORS is restrictive so it's not + vulnerable to CSRF attacks. And server_2's is using properly sanitized + user-controlled data. + +

+ + +
+ + +
  • Mozilla Developer Network: CORS, Access-Control-Allow-Origin.
  • +
  • W3C: CORS for developers, Advice for Resource Owners
  • +
    +
    diff --git a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfiguration.ql b/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfiguration.ql new file mode 100644 index 000000000000..e82d0e85ade5 --- /dev/null +++ b/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfiguration.ql @@ -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" diff --git a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationCustomizations.qll b/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationCustomizations.qll new file mode 100644 index 000000000000..045d1c1ef549 --- /dev/null +++ b/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationCustomizations.qll @@ -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 } + } +} diff --git a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationQuery.qll b/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationQuery.qll new file mode 100644 index 000000000000..4d56365aafe7 --- /dev/null +++ b/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationQuery.qll @@ -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 } +} diff --git a/javascript/ql/src/experimental/Security/CWE-942/examples/CorsPermissiveConfigurationBad.js b/javascript/ql/src/experimental/Security/CWE-942/examples/CorsPermissiveConfigurationBad.js new file mode 100644 index 000000000000..68317486a99d --- /dev/null +++ b/javascript/ql/src/experimental/Security/CWE-942/examples/CorsPermissiveConfigurationBad.js @@ -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 } + }); +}); \ No newline at end of file diff --git a/javascript/ql/src/experimental/Security/CWE-942/examples/CorsPermissiveConfigurationGood.js b/javascript/ql/src/experimental/Security/CWE-942/examples/CorsPermissiveConfigurationGood.js new file mode 100644 index 000000000000..5e084d089b75 --- /dev/null +++ b/javascript/ql/src/experimental/Security/CWE-942/examples/CorsPermissiveConfigurationGood.js @@ -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 } + }); +}); \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-942/CorsPermissiveConfiguration.expected b/javascript/ql/test/experimental/Security/CWE-942/CorsPermissiveConfiguration.expected new file mode 100644 index 000000000000..965fc4c722b4 --- /dev/null +++ b/javascript/ql/test/experimental/Security/CWE-942/CorsPermissiveConfiguration.expected @@ -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 | diff --git a/javascript/ql/test/experimental/Security/CWE-942/CorsPermissiveConfiguration.qlref b/javascript/ql/test/experimental/Security/CWE-942/CorsPermissiveConfiguration.qlref new file mode 100644 index 000000000000..1e6a39679c0d --- /dev/null +++ b/javascript/ql/test/experimental/Security/CWE-942/CorsPermissiveConfiguration.qlref @@ -0,0 +1 @@ +./experimental/Security/CWE-942/CorsPermissiveConfiguration.ql \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-942/apollo-test.js b/javascript/ql/test/experimental/Security/CWE-942/apollo-test.js new file mode 100644 index 000000000000..f55d5dc2c3ec --- /dev/null +++ b/javascript/ql/test/experimental/Security/CWE-942/apollo-test.js @@ -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 } + }); +}); \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-942/express-test.js b/javascript/ql/test/experimental/Security/CWE-942/express-test.js new file mode 100644 index 000000000000..3ad31a6a31a8 --- /dev/null +++ b/javascript/ql/test/experimental/Security/CWE-942/express-test.js @@ -0,0 +1,36 @@ +const cors = require('cors'); +var express = require('express'); + +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, default value is * + var app1 = express(); + app1.use(cors()); + + // GOOD: restrictive CORS + var app2 = express(); + var corsOptions2 = { + origin: ["https://example1.com", "https://example2.com"], + }; + app2.use(cors(corsOptions2)); + + // BAD: CORS too permissive + var app3 = express(); + var corsOption3 = { + origin: '*' + }; + app3.use(cors(corsOption3)); + + // BAD: CORS is controlled by user + var app4 = express(); + var corsOption4 = { + origin: user_origin + }; + app4.use(cors(corsOption4)); +}); \ No newline at end of file