diff --git a/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll index 96a7d712b08..4987a7fc073 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 05084202858..1699129d2a0 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 46df4b62135..4fa3ed3459a 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 9b21ed56873..31454e0a4eb 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