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..5962b8ee7d08 100644 --- a/javascript/ql/lib/ext/apollo-server.model.yml +++ b/javascript/ql/lib/ext/apollo-server.model.yml @@ -5,6 +5,12 @@ 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"] + - addsTo: pack: codeql/javascript-all extensible: typeModel @@ -13,3 +19,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/src/experimental/Security/CWE-942/Cors.qll b/javascript/ql/lib/semmle/javascript/frameworks/Cors.qll similarity index 100% rename from javascript/ql/src/experimental/Security/CWE-942/Cors.qll rename to javascript/ql/lib/semmle/javascript/frameworks/Cors.qll diff --git a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll similarity index 73% rename from javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationCustomizations.qll rename to javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll index 8876373a3d24..583847ab0d98 100644 --- a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll @@ -5,8 +5,7 @@ */ import javascript -import Cors::Cors -import Apollo::Apollo +private import semmle.javascript.frameworks.Cors /** Module containing sources, sinks, and sanitizers for overly permissive CORS configurations. */ module CorsPermissiveConfiguration { @@ -25,20 +24,10 @@ module CorsPermissiveConfiguration { 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() } @@ -64,11 +53,6 @@ module CorsPermissiveConfiguration { */ 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. */ @@ -76,20 +60,6 @@ module CorsPermissiveConfiguration { 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 } @@ -105,7 +75,8 @@ module CorsPermissiveConfiguration { */ class CorsApolloServer extends Sink, DataFlow::ValueNode { CorsApolloServer() { - exists(ApolloServer agql | + exists(API::NewNode agql | + agql = ModelOutput::getATypeNode("ApolloServer").getAnInstantiation() and this = agql.getOptionArgument(0, "cors").getALocalSource().getAPropertyWrite("origin").getRhs() ) @@ -125,7 +96,7 @@ module CorsPermissiveConfiguration { * An express route setup configured with the `cors` package. */ class CorsConfiguration extends DataFlow::MethodCallNode { - Cors corsConfig; + Cors::Cors corsConfig; CorsConfiguration() { exists(Express::RouteSetup setup | this = setup | @@ -136,6 +107,6 @@ module CorsPermissiveConfiguration { } /** Gets the expression that configures `cors` on this route setup. */ - Cors getCorsConfiguration() { result = corsConfig } + Cors::Cors getCorsConfiguration() { result = corsConfig } } } diff --git a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationQuery.qll b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll similarity index 61% rename from javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationQuery.qll rename to javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll index 3605a1adaa93..0db678e43afd 100644 --- a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll @@ -39,31 +39,3 @@ module CorsPermissiveConfigurationConfig implements DataFlow::StateConfigSig { 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/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/experimental/Security/CWE-942/CorsPermissiveConfiguration.ql b/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.ql similarity index 53% rename from javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfiguration.ql rename to javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.ql index 87db66ad98d9..050842028585 100644 --- a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfiguration.ql +++ b/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.ql @@ -1,5 +1,5 @@ /** - * @name overly CORS configuration + * @name Permissive CORS configuration * @description Misconfiguration of CORS HTTP headers allows CSRF attacks. * @kind path-problem * @problem.severity error @@ -11,11 +11,12 @@ */ import javascript -import CorsPermissiveConfigurationQuery -import CorsPermissiveConfigurationFlow::PathGraph +import semmle.javascript.security.CorsPermissiveConfigurationQuery as CorsQuery +import CorsQuery::CorsPermissiveConfigurationFlow::PathGraph from - CorsPermissiveConfigurationFlow::PathNode source, CorsPermissiveConfigurationFlow::PathNode sink -where CorsPermissiveConfigurationFlow::flowPath(source, sink) + CorsQuery::CorsPermissiveConfigurationFlow::PathNode source, + CorsQuery::CorsPermissiveConfigurationFlow::PathNode sink +where CorsQuery::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/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..112fb0c628ff --- /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 "CORS misconfiguration" (`js/cors-misconfiguration`) 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/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/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 100% rename from javascript/ql/test/experimental/Security/CWE-942/CorsPermissiveConfiguration.expected rename to javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.expected 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 84% 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..9b21ed56873b 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,14 @@ 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)); }); \ No newline at end of file