From 358617f53365db5b808bc668f0aae704e56790c2 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Wed, 30 Jul 2025 09:49:04 +0000 Subject: [PATCH 01/13] Move CORS misconfiguration query from experimental to Security --- .../semmle/javascript/frameworks}/Apollo.qll | 0 .../semmle/javascript/frameworks}/Cors.qll | 0 .../CorsPermissiveConfigurationCustomizations.qll | 10 +++++----- .../security}/CorsPermissiveConfigurationQuery.qll | 0 .../CWE-942/CorsPermissiveConfiguration.qhelp | 0 .../Security/CWE-942/CorsPermissiveConfiguration.ql | 11 ++++++----- .../examples/CorsPermissiveConfigurationBad.js | 0 .../examples/CorsPermissiveConfigurationGood.js | 0 .../CWE-942/CorsPermissiveConfiguration.qlref | 1 - .../CWE-942/CorsPermissiveConfiguration.expected | 0 .../CWE-942/CorsPermissiveConfiguration.qlref | 1 + .../Security/CWE-942/apollo-test.js | 0 .../Security/CWE-942/express-test.js | 0 13 files changed, 12 insertions(+), 11 deletions(-) rename javascript/ql/{src/experimental/Security/CWE-942 => lib/semmle/javascript/frameworks}/Apollo.qll (100%) rename javascript/ql/{src/experimental/Security/CWE-942 => lib/semmle/javascript/frameworks}/Cors.qll (100%) rename javascript/ql/{src/experimental/Security/CWE-942 => lib/semmle/javascript/security}/CorsPermissiveConfigurationCustomizations.qll (94%) rename javascript/ql/{src/experimental/Security/CWE-942 => lib/semmle/javascript/security}/CorsPermissiveConfigurationQuery.qll (100%) rename javascript/ql/src/{experimental => }/Security/CWE-942/CorsPermissiveConfiguration.qhelp (100%) rename javascript/ql/src/{experimental => }/Security/CWE-942/CorsPermissiveConfiguration.ql (53%) rename javascript/ql/src/{experimental => }/Security/CWE-942/examples/CorsPermissiveConfigurationBad.js (100%) rename javascript/ql/src/{experimental => }/Security/CWE-942/examples/CorsPermissiveConfigurationGood.js (100%) delete mode 100644 javascript/ql/test/experimental/Security/CWE-942/CorsPermissiveConfiguration.qlref rename javascript/ql/test/{experimental => query-tests}/Security/CWE-942/CorsPermissiveConfiguration.expected (100%) create mode 100644 javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.qlref rename javascript/ql/test/{experimental => query-tests}/Security/CWE-942/apollo-test.js (100%) rename javascript/ql/test/{experimental => query-tests}/Security/CWE-942/express-test.js (100%) diff --git a/javascript/ql/src/experimental/Security/CWE-942/Apollo.qll b/javascript/ql/lib/semmle/javascript/frameworks/Apollo.qll similarity index 100% rename from javascript/ql/src/experimental/Security/CWE-942/Apollo.qll rename to javascript/ql/lib/semmle/javascript/frameworks/Apollo.qll 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 94% rename from javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationCustomizations.qll rename to javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll index 8876373a3d24..b642b98b35ba 100644 --- a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll @@ -5,8 +5,8 @@ */ import javascript -import Cors::Cors -import Apollo::Apollo +private import semmle.javascript.frameworks.Apollo +private import semmle.javascript.frameworks.Cors /** Module containing sources, sinks, and sanitizers for overly permissive CORS configurations. */ module CorsPermissiveConfiguration { @@ -105,7 +105,7 @@ module CorsPermissiveConfiguration { */ class CorsApolloServer extends Sink, DataFlow::ValueNode { CorsApolloServer() { - exists(ApolloServer agql | + exists(Apollo::ApolloServer agql | this = agql.getOptionArgument(0, "cors").getALocalSource().getAPropertyWrite("origin").getRhs() ) @@ -125,7 +125,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 +136,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 100% rename from javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationQuery.qll rename to javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll diff --git a/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfiguration.qhelp b/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.qhelp similarity index 100% rename from javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfiguration.qhelp rename to javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.qhelp 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/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..4f4178905a29 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.qlref @@ -0,0 +1 @@ +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/query-tests/Security/CWE-942/apollo-test.js similarity index 100% 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 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 100% 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 From 92daa7d42cd2835a00c5e17deea4cc1a44401112 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Wed, 30 Jul 2025 10:27:14 +0000 Subject: [PATCH 02/13] Updated suite expectations --- .../query-suite/javascript-code-scanning.qls.expected | 1 + .../query-suite/javascript-security-and-quality.qls.expected | 1 + .../query-suite/javascript-security-extended.qls.expected | 1 + .../integration-tests/query-suite/not_included_in_qls.expected | 1 - 4 files changed, 3 insertions(+), 1 deletion(-) 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 From 95743d7109180c7076fe732c2e51ff1ee88d14ab Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Wed, 30 Jul 2025 10:42:55 +0000 Subject: [PATCH 03/13] Added inline test expectations for cors permissive config --- .../Security/CWE-942/CorsPermissiveConfiguration.qlref | 3 ++- .../ql/test/query-tests/Security/CWE-942/apollo-test.js | 8 ++++---- .../ql/test/query-tests/Security/CWE-942/express-test.js | 6 +++--- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.qlref b/javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.qlref index 4f4178905a29..b38b30eb842d 100644 --- a/javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.qlref +++ b/javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.qlref @@ -1 +1,2 @@ -Security/CWE-942/CorsPermissiveConfiguration.ql \ No newline at end of file +query: Security/CWE-942/CorsPermissiveConfiguration.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-942/apollo-test.js b/javascript/ql/test/query-tests/Security/CWE-942/apollo-test.js index f55d5dc2c3ec..22019a722584 100644 --- a/javascript/ql/test/query-tests/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/query-tests/Security/CWE-942/express-test.js b/javascript/ql/test/query-tests/Security/CWE-942/express-test.js index 3ad31a6a31a8..9b21ed56873b 100644 --- a/javascript/ql/test/query-tests/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 From 84ffbbec33cbff31393da847b815f2c56ae076c2 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Wed, 30 Jul 2025 10:51:38 +0000 Subject: [PATCH 04/13] Added missing doc strings --- .../security/CorsPermissiveConfigurationCustomizations.qll | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll index b642b98b35ba..4751ace2a608 100644 --- a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll @@ -26,6 +26,7 @@ module CorsPermissiveConfiguration { this = TWildcard() and result = "wildcard" } + /** DEPRECATED: Converts this flow state to a flow label. */ deprecated DataFlow::FlowLabel toFlowLabel() { this = TTaint() and result.isTaint() or @@ -37,6 +38,7 @@ module CorsPermissiveConfiguration { /** Predicates for working with flow states. */ module FlowState { + /** DEPRECATED: Gets a flow state from a flow label. */ deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } /** A tainted value. */ @@ -81,6 +83,7 @@ module CorsPermissiveConfiguration { TrueAndNull() { this = "TrueAndNull" } } + /** DEPRECATED: Gets a flow label representing `true` and `null` values. */ deprecated TrueAndNull truenullLabel() { any() } /** A flow label representing `*` value. */ @@ -88,6 +91,7 @@ module CorsPermissiveConfiguration { Wildcard() { this = "Wildcard" } } + /** DEPRECATED: Gets a flow label representing `*` value. */ deprecated Wildcard wildcardLabel() { any() } /** An overly permissive value for `origin` (Apollo) */ From fd4233e30edc5b828c53f1b4b8cfe76becf154b3 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Thu, 31 Jul 2025 10:53:03 +0200 Subject: [PATCH 05/13] Moved apollo modeling to MaD --- javascript/ql/lib/ext/apollo-server.model.yml | 12 +++++++ .../semmle/javascript/frameworks/Apollo.qll | 36 ------------------- ...sPermissiveConfigurationCustomizations.qll | 4 +-- 3 files changed, 14 insertions(+), 38 deletions(-) delete mode 100644 javascript/ql/lib/semmle/javascript/frameworks/Apollo.qll 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/lib/semmle/javascript/frameworks/Apollo.qll b/javascript/ql/lib/semmle/javascript/frameworks/Apollo.qll deleted file mode 100644 index 983c0a8ac89c..000000000000 --- a/javascript/ql/lib/semmle/javascript/frameworks/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/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll index 4751ace2a608..a504f66ba22e 100644 --- a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll @@ -5,7 +5,6 @@ */ import javascript -private import semmle.javascript.frameworks.Apollo private import semmle.javascript.frameworks.Cors /** Module containing sources, sinks, and sanitizers for overly permissive CORS configurations. */ @@ -109,7 +108,8 @@ module CorsPermissiveConfiguration { */ class CorsApolloServer extends Sink, DataFlow::ValueNode { CorsApolloServer() { - exists(Apollo::ApolloServer agql | + exists(API::NewNode agql | + agql = ModelOutput::getATypeNode("ApolloServer").getAnInstantiation() and this = agql.getOptionArgument(0, "cors").getALocalSource().getAPropertyWrite("origin").getRhs() ) From 2baca58b278827703fd803889555c71b5bd05a8e Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Thu, 31 Jul 2025 11:08:22 +0200 Subject: [PATCH 06/13] Removed deprecations from cors as it was moved out experimental --- ...sPermissiveConfigurationCustomizations.qll | 33 ------------------- .../CorsPermissiveConfigurationQuery.qll | 28 ---------------- 2 files changed, 61 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll index a504f66ba22e..583847ab0d98 100644 --- a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll @@ -24,22 +24,10 @@ module CorsPermissiveConfiguration { or this = TWildcard() and result = "wildcard" } - - /** DEPRECATED: Converts this flow state to a flow label. */ - 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: Gets a flow state from a flow label. */ - deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label } - /** A tainted value. */ FlowState taint() { result = TTaint() } @@ -65,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. */ @@ -77,22 +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: Gets a flow label representing `true` and `null` values. */ - deprecated TrueAndNull truenullLabel() { any() } - - /** A flow label representing `*` value. */ - abstract deprecated class Wildcard extends DataFlow::FlowLabel { - Wildcard() { this = "Wildcard" } - } - - /** DEPRECATED: Gets a flow label representing `*` value. */ - deprecated Wildcard wildcardLabel() { any() } - /** An overly permissive value for `origin` (Apollo) */ class TrueNullValue extends Source { TrueNullValue() { this.mayHaveBooleanValue(true) or this.asExpr() instanceof NullLiteral } diff --git a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll index 3605a1adaa93..0db678e43afd 100644 --- a/javascript/ql/lib/semmle/javascript/security/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 } -} From 791a7e242e5ca1e9ad68c21a49cd76730bdb0370 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Thu, 31 Jul 2025 11:31:10 +0200 Subject: [PATCH 07/13] Updated qhelp for cors permissive configuration --- .../CWE-942/CorsPermissiveConfiguration.qhelp | 92 ++++++++++--------- 1 file changed, 47 insertions(+), 45 deletions(-) diff --git a/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.qhelp b/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.qhelp index fc79eee743bf..04796dfbc189 100644 --- a/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.qhelp +++ b/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.qhelp @@ -3,69 +3,71 @@ "qhelp.dtd"> - -

+ +

- 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 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 lead to CSRF which is an attack that allows attackers to trick - users into performing unwanted operations in websites they're authenticated to. +

+

-

+ 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, 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. + 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. -

-

+

+

- 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. + 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. - If the origin value is user controlled, make sure that the data - is properly sanitized. +

+
-

- + +

- -

+ 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. - 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. +

-

+ - +

-

+ 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. - 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
  • -
    + +
  • Mozilla Developer Network: CORS, Access-Control-Allow-Origin.
  • +
  • W3C: CORS for developers, Advice for Resource Owners.
  • +
    From 021aa13ee2d544de36bcb26f16857c9c085401d2 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Thu, 31 Jul 2025 12:45:34 +0200 Subject: [PATCH 08/13] Added change note --- .../change-notes/2025-07-31-cors-move-out-of-experimental.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 javascript/ql/src/change-notes/2025-07-31-cors-move-out-of-experimental.md 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. From 4dac80a998ee2b9645034d3bbacf06d92fc30b73 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Thu, 4 Sep 2025 12:19:22 +0000 Subject: [PATCH 09/13] Replace complex wrapper classes with MaD --- javascript/ql/lib/ext/apollo-server.model.yml | 1 + javascript/ql/lib/ext/cors.model.yml | 6 +++ .../lib/semmle/javascript/frameworks/Cors.qll | 24 ------------ ...sPermissiveConfigurationCustomizations.qll | 38 +------------------ .../CorsPermissiveConfigurationQuery.qll | 5 +-- .../CorsPermissiveConfiguration.expected | 22 +++++++---- shared/mad/codeql/mad/ModelValidation.qll | 2 +- 7 files changed, 27 insertions(+), 71 deletions(-) create mode 100644 javascript/ql/lib/ext/cors.model.yml delete mode 100644 javascript/ql/lib/semmle/javascript/frameworks/Cors.qll diff --git a/javascript/ql/lib/ext/apollo-server.model.yml b/javascript/ql/lib/ext/apollo-server.model.yml index 5962b8ee7d08..35c6929ce264 100644 --- a/javascript/ql/lib/ext/apollo-server.model.yml +++ b/javascript/ql/lib/ext/apollo-server.model.yml @@ -10,6 +10,7 @@ extensions: extensible: sinkModel data: - ["@apollo/server", "Member[gql].Argument[0]", "sql-injection"] + - ["@apollo/server", "Member[ApolloServer,ApolloServerBase].Argument[0].Member[cors].Member[origin]", "cors-misconfiguration"] - addsTo: pack: codeql/javascript-all diff --git a/javascript/ql/lib/ext/cors.model.yml b/javascript/ql/lib/ext/cors.model.yml new file mode 100644 index 000000000000..93ec2601e4a3 --- /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-misconfiguration"] \ No newline at end of file diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Cors.qll b/javascript/ql/lib/semmle/javascript/frameworks/Cors.qll deleted file mode 100644 index cc190e6f4294..000000000000 --- a/javascript/ql/lib/semmle/javascript/frameworks/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/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll index 583847ab0d98..fdad32b966d6 100644 --- a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll @@ -5,7 +5,6 @@ */ import javascript -private import semmle.javascript.frameworks.Cors /** Module containing sources, sinks, and sanitizers for overly permissive CORS configurations. */ module CorsPermissiveConfiguration { @@ -73,40 +72,7 @@ module CorsPermissiveConfiguration { /** * The value of cors origin when initializing the application. */ - class CorsApolloServer extends Sink, DataFlow::ValueNode { - CorsApolloServer() { - exists(API::NewNode agql | - agql = ModelOutput::getATypeNode("ApolloServer").getAnInstantiation() and - 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::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::Cors getCorsConfiguration() { result = corsConfig } + class CorsOriginSink extends Sink, DataFlow::ValueNode { + CorsOriginSink() { this = ModelOutput::getASinkNode("cors-misconfiguration").asSink() } } } diff --git a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll index 0db678e43afd..e0d8e2d644cd 100644 --- a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll @@ -27,9 +27,8 @@ module CorsPermissiveConfigurationConfig implements DataFlow::StateConfigSig { } 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()] + sink instanceof CorsOriginSink and + state = [FlowState::taint(), FlowState::trueOrNull(), FlowState::wildcard()] } predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } diff --git a/javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.expected b/javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.expected index 6c28b7105a18..46df4b621352 100644 --- a/javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.expected +++ b/javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.expected @@ -1,3 +1,11 @@ +#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 | +| 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 misconfiguration due to a $@. | express-test.js:10:42:10:45 | true | too permissive or user controlled value | edges | apollo-test.js:8:9:8:59 | user_origin | apollo-test.js:26:25:26:35 | user_origin | provenance | | | apollo-test.js:8:9:8:59 | user_origin | apollo-test.js:26:25:26:35 | user_origin | provenance | | @@ -6,8 +14,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:59 | user_origin | express-test.js:33:17:33:27 | user_origin | provenance | | +| express-test.js:10:9:10:59 | 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:59 | user_origin | provenance | | | express-test.js:10:23:10:46 | url.par ... , true) | express-test.js:10:9:10:59 | 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:59 | user_origin | semmle.label | user_origin | | apollo-test.js:8:9:8:59 | user_origin | semmle.label | user_origin | @@ -20,15 +31,12 @@ 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:59 | user_origin | semmle.label | user_origin | +| express-test.js:10:9:10:59 | 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 | 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/shared/mad/codeql/mad/ModelValidation.qll b/shared/mad/codeql/mad/ModelValidation.qll index 018c1797ddcd..a0062df6b9a6 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-misconfiguration", "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", From 6c751ce934617b6fa6d22bdbdd9bb412e2501080 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Thu, 4 Sep 2025 12:31:24 +0000 Subject: [PATCH 10/13] Merged config classes --- ...sPermissiveConfigurationCustomizations.qll | 31 +++++++------------ .../CorsPermissiveConfigurationQuery.qll | 6 ++-- 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll index fdad32b966d6..96a7d712b08a 100644 --- a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll @@ -10,18 +10,15 @@ import javascript module CorsPermissiveConfiguration { private newtype TFlowState = TTaint() or - TTrueOrNull() or - TWildcard() + TPermissive() - /** A flow state to asociate with a tracked value. */ + /** 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 = TTrueOrNull() and result = "true-or-null" - or - this = TWildcard() and result = "wildcard" + this = TPermissive() and result = "permissive" } } @@ -30,11 +27,8 @@ module CorsPermissiveConfiguration { /** A tainted value. */ FlowState taint() { result = TTaint() } - /** A `true` or `null` value. */ - FlowState trueOrNull() { result = TTrueOrNull() } - - /** A `"*"` value. */ - FlowState wildcard() { result = TWildcard() } + /** A permissive value (true, null, or "*"). */ + FlowState permissive() { result = TPermissive() } } /** @@ -59,14 +53,13 @@ module CorsPermissiveConfiguration { ActiveThreatModelSourceAsSource() { not this instanceof ClientSideRemoteFlowSource } } - /** 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("*") } + /** An overly permissive value for `origin` configuration. */ + class PermissiveValue extends Source { + PermissiveValue() { + this.mayHaveBooleanValue(true) or + this.asExpr() instanceof NullLiteral or + this.mayHaveStringValue("*") + } } /** diff --git a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll index e0d8e2d644cd..03d20578b0e1 100644 --- a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll @@ -19,16 +19,14 @@ 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() + 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::trueOrNull(), FlowState::wildcard()] + state = [FlowState::taint(), FlowState::permissive()] } predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } From d3d608fa33606cc72b0a9b9fec472e3a9723206b Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Thu, 4 Sep 2025 13:16:37 +0000 Subject: [PATCH 11/13] Updated query description and added a sanitizer --- ...CorsPermissiveConfigurationCustomizations.qll | 14 ++++++++++++++ .../CWE-942/CorsPermissiveConfiguration.ql | 12 ++++++------ .../CWE-942/CorsPermissiveConfiguration.expected | 16 +++++++++------- .../query-tests/Security/CWE-942/express-test.js | 16 ++++++++++++++++ 4 files changed, 45 insertions(+), 13 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll index 96a7d712b08a..4987a7fc073e 100644 --- a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll @@ -68,4 +68,18 @@ module CorsPermissiveConfiguration { class CorsOriginSink extends Sink, DataFlow::ValueNode { CorsOriginSink() { this = ModelOutput::getASinkNode("cors-misconfiguration").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/src/Security/CWE-942/CorsPermissiveConfiguration.ql b/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.ql index 050842028585..1699129d2a05 100644 --- a/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.ql +++ b/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.ql @@ -1,11 +1,11 @@ /** * @name Permissive CORS configuration - * @description Misconfiguration of CORS HTTP headers allows CSRF attacks. + * @description Cross-origin resource sharing (CORS) policy allows overly broad access. * @kind path-problem - * @problem.severity error - * @security-severity 7.5 + * @problem.severity warning + * @security-severity 6.0 * @precision high - * @id js/cors-misconfiguration + * @id js/cors-permissive-configuration * @tags security * external/cwe/cwe-942 */ @@ -18,5 +18,5 @@ from 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" +select sink.getNode(), source, sink, "CORS Origin allows broad access due to $@.", source.getNode(), + "permissive or user controlled value" diff --git a/javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.expected b/javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.expected index 46df4b621352..4fa3ed3459a7 100644 --- a/javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.expected +++ b/javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.expected @@ -1,11 +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 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 | -| 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 misconfiguration due to a $@. | express-test.js:10:42:10:45 | true | too permissive or user controlled value | +| 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:59 | user_origin | apollo-test.js:26:25:26:35 | user_origin | provenance | | | apollo-test.js:8:9:8:59 | user_origin | apollo-test.js:26:25:26:35 | user_origin | provenance | | @@ -39,4 +40,5 @@ nodes | 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 diff --git a/javascript/ql/test/query-tests/Security/CWE-942/express-test.js b/javascript/ql/test/query-tests/Security/CWE-942/express-test.js index 9b21ed56873b..31454e0a4ebf 100644 --- a/javascript/ql/test/query-tests/Security/CWE-942/express-test.js +++ b/javascript/ql/test/query-tests/Security/CWE-942/express-test.js @@ -33,4 +33,20 @@ server.on('request', function (req, res) { 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 From e6eacca50b95fcbbe62edbed5e247ffd3db13c2b Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Fri, 5 Sep 2025 11:27:29 +0200 Subject: [PATCH 12/13] Update change note to reflect changes --- .../change-notes/2025-07-31-cors-move-out-of-experimental.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 112fb0c628ff..db04cbc7d930 100644 --- 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 @@ -1,4 +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. +* The query "Permissive CORS configuration" (`js/cors-permissive-configuration`) has been promoted from experimental and is now part of the default security suite. From d8c4d6deb4c2825e05c7b300c92d935d33f4a33a Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Fri, 5 Sep 2025 11:30:07 +0200 Subject: [PATCH 13/13] Rename `cors-misconfiguration` to `cors-origin`. --- javascript/ql/lib/ext/apollo-server.model.yml | 2 +- javascript/ql/lib/ext/cors.model.yml | 2 +- .../security/CorsPermissiveConfigurationCustomizations.qll | 2 +- shared/mad/codeql/mad/ModelValidation.qll | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/javascript/ql/lib/ext/apollo-server.model.yml b/javascript/ql/lib/ext/apollo-server.model.yml index 35c6929ce264..aa9755265b27 100644 --- a/javascript/ql/lib/ext/apollo-server.model.yml +++ b/javascript/ql/lib/ext/apollo-server.model.yml @@ -10,7 +10,7 @@ extensions: extensible: sinkModel data: - ["@apollo/server", "Member[gql].Argument[0]", "sql-injection"] - - ["@apollo/server", "Member[ApolloServer,ApolloServerBase].Argument[0].Member[cors].Member[origin]", "cors-misconfiguration"] + - ["@apollo/server", "Member[ApolloServer,ApolloServerBase].Argument[0].Member[cors].Member[origin]", "cors-origin"] - addsTo: pack: codeql/javascript-all diff --git a/javascript/ql/lib/ext/cors.model.yml b/javascript/ql/lib/ext/cors.model.yml index 93ec2601e4a3..31125d454b9b 100644 --- a/javascript/ql/lib/ext/cors.model.yml +++ b/javascript/ql/lib/ext/cors.model.yml @@ -3,4 +3,4 @@ extensions: pack: codeql/javascript-all extensible: sinkModel data: - - ["cors", "Argument[0].Member[origin]", "cors-misconfiguration"] \ No newline at end of file + - ["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 index 4987a7fc073e..aed0e26a6b3f 100644 --- a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll @@ -66,7 +66,7 @@ module CorsPermissiveConfiguration { * The value of cors origin when initializing the application. */ class CorsOriginSink extends Sink, DataFlow::ValueNode { - CorsOriginSink() { this = ModelOutput::getASinkNode("cors-misconfiguration").asSink() } + CorsOriginSink() { this = ModelOutput::getASinkNode("cors-origin").asSink() } } /** diff --git a/shared/mad/codeql/mad/ModelValidation.qll b/shared/mad/codeql/mad/ModelValidation.qll index a0062df6b9a6..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 - "cors-misconfiguration", "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",