diff --git a/javascript/ql/integration-tests/query-suite/javascript-code-scanning.qls.expected b/javascript/ql/integration-tests/query-suite/javascript-code-scanning.qls.expected index 652ac0ebc1b9..0c417e661c79 100644 --- a/javascript/ql/integration-tests/query-suite/javascript-code-scanning.qls.expected +++ b/javascript/ql/integration-tests/query-suite/javascript-code-scanning.qls.expected @@ -83,5 +83,6 @@ ql/javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql ql/javascript/ql/src/Security/CWE-915/PrototypePollutingMergeCall.ql ql/javascript/ql/src/Security/CWE-916/InsufficientPasswordHash.ql ql/javascript/ql/src/Security/CWE-918/RequestForgery.ql +ql/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.ql ql/javascript/ql/src/Summary/LinesOfCode.ql ql/javascript/ql/src/Summary/LinesOfUserCode.ql diff --git a/javascript/ql/integration-tests/query-suite/javascript-security-and-quality.qls.expected b/javascript/ql/integration-tests/query-suite/javascript-security-and-quality.qls.expected index dd5877683082..f87cd2bf505a 100644 --- a/javascript/ql/integration-tests/query-suite/javascript-security-and-quality.qls.expected +++ b/javascript/ql/integration-tests/query-suite/javascript-security-and-quality.qls.expected @@ -184,6 +184,7 @@ ql/javascript/ql/src/Security/CWE-915/PrototypePollutingMergeCall.ql ql/javascript/ql/src/Security/CWE-916/InsufficientPasswordHash.ql ql/javascript/ql/src/Security/CWE-918/ClientSideRequestForgery.ql ql/javascript/ql/src/Security/CWE-918/RequestForgery.ql +ql/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.ql ql/javascript/ql/src/Statements/DanglingElse.ql ql/javascript/ql/src/Statements/IgnoreArrayResult.ql ql/javascript/ql/src/Statements/InconsistentLoopOrientation.ql diff --git a/javascript/ql/integration-tests/query-suite/javascript-security-extended.qls.expected b/javascript/ql/integration-tests/query-suite/javascript-security-extended.qls.expected index 9b7cfd22ed6f..ac5e0e2c4984 100644 --- a/javascript/ql/integration-tests/query-suite/javascript-security-extended.qls.expected +++ b/javascript/ql/integration-tests/query-suite/javascript-security-extended.qls.expected @@ -99,5 +99,6 @@ ql/javascript/ql/src/Security/CWE-915/PrototypePollutingMergeCall.ql ql/javascript/ql/src/Security/CWE-916/InsufficientPasswordHash.ql ql/javascript/ql/src/Security/CWE-918/ClientSideRequestForgery.ql ql/javascript/ql/src/Security/CWE-918/RequestForgery.ql +ql/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.ql ql/javascript/ql/src/Summary/LinesOfCode.ql ql/javascript/ql/src/Summary/LinesOfUserCode.ql diff --git a/javascript/ql/integration-tests/query-suite/not_included_in_qls.expected b/javascript/ql/integration-tests/query-suite/not_included_in_qls.expected index 1b119f60c75e..fa52a97a4e4a 100644 --- a/javascript/ql/integration-tests/query-suite/not_included_in_qls.expected +++ b/javascript/ql/integration-tests/query-suite/not_included_in_qls.expected @@ -75,7 +75,6 @@ ql/javascript/ql/src/experimental/Security/CWE-347/decodeJwtWithoutVerificationL ql/javascript/ql/src/experimental/Security/CWE-444/InsecureHttpParser.ql ql/javascript/ql/src/experimental/Security/CWE-522-DecompressionBombs/DecompressionBombs.ql ql/javascript/ql/src/experimental/Security/CWE-918/SSRF.ql -ql/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfiguration.ql ql/javascript/ql/src/experimental/StandardLibrary/MultipleArgumentsToSetConstructor.ql ql/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.ql ql/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-078/CommandInjection.ql diff --git a/javascript/ql/lib/ext/apollo-server.model.yml b/javascript/ql/lib/ext/apollo-server.model.yml index ffceb6a6d5af..aa9755265b27 100644 --- a/javascript/ql/lib/ext/apollo-server.model.yml +++ b/javascript/ql/lib/ext/apollo-server.model.yml @@ -5,6 +5,13 @@ extensions: data: - ["@apollo/server", "Member[ApolloServer,ApolloServerBase].Argument[0].AnyMember.AnyMember.AnyMember.Parameter[1]", "remote"] + - addsTo: + pack: codeql/javascript-all + extensible: sinkModel + data: + - ["@apollo/server", "Member[gql].Argument[0]", "sql-injection"] + - ["@apollo/server", "Member[ApolloServer,ApolloServerBase].Argument[0].Member[cors].Member[origin]", "cors-origin"] + - addsTo: pack: codeql/javascript-all extensible: typeModel @@ -13,3 +20,9 @@ extensions: - ["@apollo/server", "apollo-server-express", ""] - ["@apollo/server", "apollo-server-core", ""] - ["@apollo/server", "apollo-server", ""] + - ["@apollo/server", "@apollo/apollo-server-express", ""] + - ["@apollo/server", "apollo-server-express", ""] + - ["@apollo/server", "@apollo/server", ""] + - ["@apollo/server", "@apollo/apollo-server-core", ""] + - ["ApolloServer", "@apollo/server", "Member[ApolloServer]"] + - ["GraphQLApollo", "@apollo/server", "Member[gql]"] diff --git a/javascript/ql/lib/ext/cors.model.yml b/javascript/ql/lib/ext/cors.model.yml new file mode 100644 index 000000000000..31125d454b9b --- /dev/null +++ b/javascript/ql/lib/ext/cors.model.yml @@ -0,0 +1,6 @@ +extensions: + - addsTo: + pack: codeql/javascript-all + extensible: sinkModel + data: + - ["cors", "Argument[0].Member[origin]", "cors-origin"] diff --git a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll new file mode 100644 index 000000000000..aed0e26a6b3f --- /dev/null +++ b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll @@ -0,0 +1,85 @@ +/** + * 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 { + private newtype TFlowState = + TTaint() or + TPermissive() + + /** A flow state to associate with a tracked value. */ + class FlowState extends TFlowState { + /** Gets a string representation of this flow state. */ + string toString() { + this = TTaint() and result = "taint" + or + this = TPermissive() and result = "permissive" + } + } + + /** Predicates for working with flow states. */ + module FlowState { + /** A tainted value. */ + FlowState taint() { result = TTaint() } + + /** A permissive value (true, null, or "*"). */ + FlowState permissive() { result = TPermissive() } + } + + /** + * 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 { } + + /** + * An active threat-model source, considered as a flow source. + */ + private class ActiveThreatModelSourceAsSource extends Source instanceof ActiveThreatModelSource { + ActiveThreatModelSourceAsSource() { not this instanceof ClientSideRemoteFlowSource } + } + + /** An overly permissive value for `origin` configuration. */ + class PermissiveValue extends Source { + PermissiveValue() { + this.mayHaveBooleanValue(true) or + this.asExpr() instanceof NullLiteral or + this.mayHaveStringValue("*") + } + } + + /** + * The value of cors origin when initializing the application. + */ + class CorsOriginSink extends Sink, DataFlow::ValueNode { + CorsOriginSink() { this = ModelOutput::getASinkNode("cors-origin").asSink() } + } + + /** + * A sanitizer for CORS configurations where credentials are explicitly disabled. + * When credentials are false, using "*" for origin is a legitimate pattern. + */ + private class CredentialsDisabledSanitizer extends Sanitizer { + CredentialsDisabledSanitizer() { + exists(DataFlow::SourceNode config, DataFlow::CallNode call | + call.getArgument(0).getALocalSource() = config and + this = config.getAPropertyWrite("origin").getRhs() and + config.getAPropertyWrite("credentials").getRhs().mayHaveBooleanValue(false) + ) + } + } +} diff --git a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll new file mode 100644 index 000000000000..03d20578b0e1 --- /dev/null +++ b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll @@ -0,0 +1,38 @@ +/** + * 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 +private import CorsPermissiveConfigurationCustomizations::CorsPermissiveConfiguration as CorsPermissiveConfiguration + +/** + * A data flow configuration for overly permissive CORS configuration. + */ +module CorsPermissiveConfigurationConfig implements DataFlow::StateConfigSig { + class FlowState = CorsPermissiveConfiguration::FlowState; + + predicate isSource(DataFlow::Node source, FlowState state) { + source instanceof PermissiveValue and state = FlowState::permissive() + or + source instanceof RemoteFlowSource and state = FlowState::taint() + } + + predicate isSink(DataFlow::Node sink, FlowState state) { + sink instanceof CorsOriginSink and + state = [FlowState::taint(), FlowState::permissive()] + } + + predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } + + predicate observeDiffInformedIncrementalMode() { any() } +} + +module CorsPermissiveConfigurationFlow = + TaintTracking::GlobalWithState; diff --git a/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.qhelp b/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.qhelp new file mode 100644 index 000000000000..04796dfbc189 --- /dev/null +++ b/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.qhelp @@ -0,0 +1,73 @@ + + + + +

+ + A server can use CORS (Cross-Origin Resource Sharing) to relax the + restrictions imposed by the 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 enable CSRF attacks, which allow attackers to trick + users into performing unwanted operations on websites they're authenticated to. + +

+ + + +

+ + When the origin is set to true, the server + accepts requests from any origin, potentially exposing the system to + CSRF attacks. Use false as the origin value or implement a whitelist + of allowed origins instead. + +

+

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

+

+ + If the origin value is user-controlled, ensure that the data + is properly sanitized and validated against a whitelist of allowed origins. + +

+ + + +

+ + In the following example, server_1 accepts requests from any origin + because the value of origin is set to true. + server_2 uses user-controlled data for the origin without validation. + +

+ + + +

+ + To fix these issues, server_1 uses a restrictive CORS configuration + that is not vulnerable to CSRF attacks. server_2 properly validates + user-controlled data against a whitelist before using it. + +

+ + + + + +
  • Mozilla Developer Network: CORS, Access-Control-Allow-Origin.
    • +
  • W3C: CORS for developers, Advice for Resource Owners.
    • +     +     diff --git a/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.ql b/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.ql new file mode 100644 index 000000000000..1699129d2a05 --- /dev/null +++ b/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.ql @@ -0,0 +1,22 @@ +/** + * @name Permissive CORS configuration + * @description Cross-origin resource sharing (CORS) policy allows overly broad access. + * @kind path-problem + * @problem.severity warning + * @security-severity 6.0 + * @precision high + * @id js/cors-permissive-configuration + * @tags security + * external/cwe/cwe-942 + */ + +import javascript +import semmle.javascript.security.CorsPermissiveConfigurationQuery as CorsQuery +import CorsQuery::CorsPermissiveConfigurationFlow::PathGraph + +from + CorsQuery::CorsPermissiveConfigurationFlow::PathNode source, + CorsQuery::CorsPermissiveConfigurationFlow::PathNode sink +where CorsQuery::CorsPermissiveConfigurationFlow::flowPath(source, sink) +select sink.getNode(), source, sink, "CORS Origin allows broad access due to $@.", source.getNode(), + "permissive or user controlled value" diff --git a/javascript/ql/src/experimental/Security/CWE-942/examples/CorsPermissiveConfigurationBad.js b/javascript/ql/src/Security/CWE-942/examples/CorsPermissiveConfigurationBad.js similarity index 100% rename from javascript/ql/src/experimental/Security/CWE-942/examples/CorsPermissiveConfigurationBad.js rename to javascript/ql/src/Security/CWE-942/examples/CorsPermissiveConfigurationBad.js diff --git a/javascript/ql/src/experimental/Security/CWE-942/examples/CorsPermissiveConfigurationGood.js b/javascript/ql/src/Security/CWE-942/examples/CorsPermissiveConfigurationGood.js similarity index 100% rename from javascript/ql/src/experimental/Security/CWE-942/examples/CorsPermissiveConfigurationGood.js rename to javascript/ql/src/Security/CWE-942/examples/CorsPermissiveConfigurationGood.js diff --git a/javascript/ql/src/change-notes/2025-07-31-cors-move-out-of-experimental.md b/javascript/ql/src/change-notes/2025-07-31-cors-move-out-of-experimental.md new file mode 100644 index 000000000000..db04cbc7d930 --- /dev/null +++ b/javascript/ql/src/change-notes/2025-07-31-cors-move-out-of-experimental.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The query "Permissive CORS configuration" (`js/cors-permissive-configuration`) has been promoted from experimental and is now part of the default security suite. diff --git a/javascript/ql/src/experimental/Security/CWE-942/Apollo.qll b/javascript/ql/src/experimental/Security/CWE-942/Apollo.qll deleted file mode 100644 index 983c0a8ac89c..000000000000 --- a/javascript/ql/src/experimental/Security/CWE-942/Apollo.qll +++ /dev/null @@ -1,36 +0,0 @@ -/** - * 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 deleted file mode 100644 index cc190e6f4294..000000000000 --- a/javascript/ql/src/experimental/Security/CWE-942/Cors.qll +++ /dev/null @@ -1,24 +0,0 @@ -/** - * 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 deleted file mode 100644 index fc79eee743bf..000000000000 --- a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfiguration.qhelp +++ /dev/null @@ -1,71 +0,0 @@ - - - - -

    - - 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 deleted file mode 100644 index 87db66ad98d9..000000000000 --- a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfiguration.ql +++ /dev/null @@ -1,21 +0,0 @@ -/** - * @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 CorsPermissiveConfigurationFlow::PathGraph - -from - CorsPermissiveConfigurationFlow::PathNode source, CorsPermissiveConfigurationFlow::PathNode sink -where CorsPermissiveConfigurationFlow::flowPath(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 deleted file mode 100644 index 8876373a3d24..000000000000 --- a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationCustomizations.qll +++ /dev/null @@ -1,141 +0,0 @@ -/** - * 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 { - private newtype TFlowState = - TTaint() or - TTrueOrNull() or - TWildcard() - - /** A flow state to asociate with a tracked value. */ - class FlowState extends TFlowState { - /** Gets a string representation of this flow state. */ - string toString() { - this = TTaint() and result = "taint" - or - this = TTrueOrNull() and result = "true-or-null" - or - this = TWildcard() and result = "wildcard" - } - - deprecated DataFlow::FlowLabel toFlowLabel() { - this = TTaint() and result.isTaint() - or - this = TTrueOrNull() and result instanceof TrueAndNull - or - this = TWildcard() and result instanceof Wildcard - } - } - - /** Predicates for working with flow states. */ - module FlowState { - deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } - - /** A tainted value. */ - FlowState taint() { result = TTaint() } - - /** A `true` or `null` value. */ - FlowState trueOrNull() { result = TTrueOrNull() } - - /** A `"*"` value. */ - FlowState wildcard() { result = TWildcard() } - } - - /** - * 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 { } - - /** - * DEPRECATED: Use `ActiveThreatModelSource` from Concepts instead! - */ - deprecated class RemoteFlowSourceAsSource = ActiveThreatModelSourceAsSource; - - /** - * An active threat-model source, considered as a flow source. - */ - private class ActiveThreatModelSourceAsSource extends Source instanceof ActiveThreatModelSource { - ActiveThreatModelSourceAsSource() { not this instanceof ClientSideRemoteFlowSource } - } - - /** A flow label representing `true` and `null` values. */ - abstract deprecated class TrueAndNull extends DataFlow::FlowLabel { - TrueAndNull() { this = "TrueAndNull" } - } - - deprecated TrueAndNull truenullLabel() { any() } - - /** A flow label representing `*` value. */ - abstract deprecated class Wildcard extends DataFlow::FlowLabel { - Wildcard() { this = "Wildcard" } - } - - deprecated 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 deleted file mode 100644 index 3605a1adaa93..000000000000 --- a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationQuery.qll +++ /dev/null @@ -1,69 +0,0 @@ -/** - * 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 -private import CorsPermissiveConfigurationCustomizations::CorsPermissiveConfiguration as CorsPermissiveConfiguration - -/** - * A data flow configuration for overly permissive CORS configuration. - */ -module CorsPermissiveConfigurationConfig implements DataFlow::StateConfigSig { - class FlowState = CorsPermissiveConfiguration::FlowState; - - predicate isSource(DataFlow::Node source, FlowState state) { - source instanceof TrueNullValue and state = FlowState::trueOrNull() - or - source instanceof WildcardValue and state = FlowState::wildcard() - or - source instanceof RemoteFlowSource and state = FlowState::taint() - } - - predicate isSink(DataFlow::Node sink, FlowState state) { - sink instanceof CorsApolloServer and state = [FlowState::taint(), FlowState::trueOrNull()] - or - sink instanceof ExpressCors and state = [FlowState::taint(), FlowState::wildcard()] - } - - predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } - - predicate observeDiffInformedIncrementalMode() { any() } -} - -module CorsPermissiveConfigurationFlow = - TaintTracking::GlobalWithState; - -/** - * DEPRECATED. Use the `CorsPermissiveConfigurationFlow` module instead. - */ -deprecated class Configuration extends TaintTracking::Configuration { - Configuration() { this = "CorsPermissiveConfiguration" } - - override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { - CorsPermissiveConfigurationConfig::isSource(source, FlowState::fromFlowLabel(label)) - } - - override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - CorsPermissiveConfigurationConfig::isSink(sink, FlowState::fromFlowLabel(label)) - } - - override predicate isSanitizer(DataFlow::Node node) { - super.isSanitizer(node) or - CorsPermissiveConfigurationConfig::isBarrier(node) - } -} - -deprecated private class WildcardActivated extends DataFlow::FlowLabel, Wildcard { - WildcardActivated() { this = this } -} - -deprecated private class TrueAndNullActivated extends DataFlow::FlowLabel, TrueAndNull { - TrueAndNullActivated() { this = this } -} diff --git a/javascript/ql/test/experimental/Security/CWE-942/CorsPermissiveConfiguration.qlref b/javascript/ql/test/experimental/Security/CWE-942/CorsPermissiveConfiguration.qlref deleted file mode 100644 index 1e6a39679c0d..000000000000 --- a/javascript/ql/test/experimental/Security/CWE-942/CorsPermissiveConfiguration.qlref +++ /dev/null @@ -1 +0,0 @@ -./experimental/Security/CWE-942/CorsPermissiveConfiguration.ql \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-942/CorsPermissiveConfiguration.expected b/javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.expected similarity index 56% rename from javascript/ql/test/experimental/Security/CWE-942/CorsPermissiveConfiguration.expected rename to javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.expected index ddebfa1d1c8a..2e59a7a8bd1d 100644 --- a/javascript/ql/test/experimental/Security/CWE-942/CorsPermissiveConfiguration.expected +++ b/javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.expected @@ -1,3 +1,12 @@ +#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 allows broad access due to $@. | apollo-test.js:11:25:11:28 | true | 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 allows broad access due to $@. | apollo-test.js:21:25:21:28 | null | 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 allows broad access due to $@. | apollo-test.js:8:33:8:39 | req.url | permissive or user controlled value | +| apollo-test.js:26:25:26:35 | user_origin | apollo-test.js:8:42:8:45 | true | apollo-test.js:26:25:26:35 | user_origin | CORS Origin allows broad access due to $@. | apollo-test.js:8:42:8:45 | true | 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 allows broad access due to $@. | express-test.js:26:17:26:19 | '*' | 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 allows broad access due to $@. | express-test.js:10:33:10:39 | req.url | permissive or user controlled value | +| express-test.js:33:17:33:27 | user_origin | express-test.js:10:42:10:45 | true | express-test.js:33:17:33:27 | user_origin | CORS Origin allows broad access due to $@. | express-test.js:10:42:10:45 | true | permissive or user controlled value | +| express-test.js:48:17:48:19 | '*' | express-test.js:48:17:48:19 | '*' | express-test.js:48:17:48:19 | '*' | CORS Origin allows broad access due to $@. | express-test.js:48:17:48:19 | '*' | permissive or user controlled value | edges | apollo-test.js:8:9:8:19 | user_origin | apollo-test.js:26:25:26:35 | user_origin | provenance | | | apollo-test.js:8:9:8:19 | user_origin | apollo-test.js:26:25:26:35 | user_origin | provenance | | @@ -6,8 +15,11 @@ edges | apollo-test.js:8:33:8:39 | req.url | apollo-test.js:8:23:8:46 | url.par ... , true) | provenance | | | apollo-test.js:8:42:8:45 | true | apollo-test.js:8:23:8:46 | url.par ... , true) | provenance | | | express-test.js:10:9:10:19 | user_origin | express-test.js:33:17:33:27 | user_origin | provenance | | +| express-test.js:10:9:10:19 | user_origin | express-test.js:33:17:33:27 | user_origin | provenance | | +| express-test.js:10:23:10:46 | url.par ... , true) | express-test.js:10:9:10:19 | user_origin | provenance | | | express-test.js:10:23:10:46 | url.par ... , true) | express-test.js:10:9:10:19 | user_origin | provenance | | | express-test.js:10:33:10:39 | req.url | express-test.js:10:23:10:46 | url.par ... , true) | provenance | | +| express-test.js:10:42:10:45 | true | express-test.js:10:23:10:46 | url.par ... , true) | provenance | | nodes | apollo-test.js:8:9:8:19 | user_origin | semmle.label | user_origin | | apollo-test.js:8:9:8:19 | user_origin | semmle.label | user_origin | @@ -20,15 +32,13 @@ nodes | apollo-test.js:26:25:26:35 | user_origin | semmle.label | user_origin | | apollo-test.js:26:25:26:35 | user_origin | semmle.label | user_origin | | express-test.js:10:9:10:19 | user_origin | semmle.label | user_origin | +| express-test.js:10:9:10:19 | user_origin | semmle.label | user_origin | +| express-test.js:10:23:10:46 | url.par ... , true) | semmle.label | url.par ... , true) | | express-test.js:10:23:10:46 | url.par ... , true) | semmle.label | url.par ... , true) | | express-test.js:10:33:10:39 | req.url | semmle.label | req.url | +| express-test.js:10:42:10:45 | true | semmle.label | true | | express-test.js:26:17:26:19 | '*' | semmle.label | '*' | | express-test.js:33:17:33:27 | user_origin | semmle.label | user_origin | +| express-test.js:33:17:33:27 | user_origin | semmle.label | user_origin | +| express-test.js:48:17:48:19 | '*' | semmle.label | '*' | subpaths -#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 | -| apollo-test.js:26:25:26:35 | user_origin | apollo-test.js:8:42:8:45 | true | apollo-test.js:26:25:26:35 | user_origin | CORS Origin misconfiguration due to a $@. | apollo-test.js:8:42:8:45 | true | 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/query-tests/Security/CWE-942/CorsPermissiveConfiguration.qlref b/javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.qlref new file mode 100644 index 000000000000..b38b30eb842d --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.qlref @@ -0,0 +1,2 @@ +query: Security/CWE-942/CorsPermissiveConfiguration.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-942/apollo-test.js b/javascript/ql/test/query-tests/Security/CWE-942/apollo-test.js similarity index 72% rename from javascript/ql/test/experimental/Security/CWE-942/apollo-test.js rename to javascript/ql/test/query-tests/Security/CWE-942/apollo-test.js index f55d5dc2c3ec..22019a722584 100644 --- a/javascript/ql/test/experimental/Security/CWE-942/apollo-test.js +++ b/javascript/ql/test/query-tests/Security/CWE-942/apollo-test.js @@ -5,10 +5,10 @@ var https = require('https'), var server = https.createServer(function () { }); server.on('request', function (req, res) { - let user_origin = url.parse(req.url, true).query.origin; + let user_origin = url.parse(req.url, true).query.origin; // $ Source // BAD: CORS too permissive const server_1 = new ApolloServer({ - cors: { origin: true } + cors: { origin: true } // $ Alert }); // GOOD: restrictive CORS @@ -18,11 +18,11 @@ server.on('request', function (req, res) { // BAD: CORS too permissive const server_3 = new ApolloServer({ - cors: { origin: null } + cors: { origin: null } // $ Alert }); // BAD: CORS is controlled by user const server_4 = new ApolloServer({ - cors: { origin: user_origin } + cors: { origin: user_origin } // $ Alert }); }); \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-942/express-test.js b/javascript/ql/test/query-tests/Security/CWE-942/express-test.js similarity index 56% rename from javascript/ql/test/experimental/Security/CWE-942/express-test.js rename to javascript/ql/test/query-tests/Security/CWE-942/express-test.js index 3ad31a6a31a8..31454e0a4ebf 100644 --- a/javascript/ql/test/experimental/Security/CWE-942/express-test.js +++ b/javascript/ql/test/query-tests/Security/CWE-942/express-test.js @@ -7,7 +7,7 @@ var https = require('https'), var server = https.createServer(function () { }); server.on('request', function (req, res) { - let user_origin = url.parse(req.url, true).query.origin; + let user_origin = url.parse(req.url, true).query.origin; // $ Source // BAD: CORS too permissive, default value is * var app1 = express(); @@ -23,14 +23,30 @@ server.on('request', function (req, res) { // BAD: CORS too permissive var app3 = express(); var corsOption3 = { - origin: '*' + origin: '*' // $ Alert }; app3.use(cors(corsOption3)); // BAD: CORS is controlled by user var app4 = express(); var corsOption4 = { - origin: user_origin + origin: user_origin // $ Alert }; app4.use(cors(corsOption4)); + + // GOOD: CORS allows any origin but credentials are disabled (safe pattern) + var app5 = express(); + var corsOption5 = { + origin: '*', + credentials: false + }; + app5.use(cors(corsOption5)); + + // BAD: CORS allows any origin with credentials enabled + var app6 = express(); + var corsOption6 = { + origin: '*', // $ Alert + credentials: true + }; + app6.use(cors(corsOption6)); }); \ No newline at end of file diff --git a/shared/mad/codeql/mad/ModelValidation.qll b/shared/mad/codeql/mad/ModelValidation.qll index 018c1797ddcd..5d4698bed1d4 100644 --- a/shared/mad/codeql/mad/ModelValidation.qll +++ b/shared/mad/codeql/mad/ModelValidation.qll @@ -39,7 +39,7 @@ module KindValidation { "response-splitting", "trust-boundary-violation", "template-injection", "url-forward", "xslt-injection", // JavaScript-only currently, but may be shared in the future - "mongodb.sink", + "cors-origin", "mongodb.sink", // Swift-only currently, but may be shared in the future "database-store", "format-string", "hash-iteration-count", "predicate-injection", "preferences-store", "tls-protocol-version", "transmission", "webview-fetch", "xxe",