Skip to content

Commit d3d608f

Browse files
committed
Updated query description and added a sanitizer
1 parent 6c751ce commit d3d608f

File tree

4 files changed

+45
-13
lines changed

4 files changed

+45
-13
lines changed

javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationCustomizations.qll

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,4 +68,18 @@ module CorsPermissiveConfiguration {
6868
class CorsOriginSink extends Sink, DataFlow::ValueNode {
6969
CorsOriginSink() { this = ModelOutput::getASinkNode("cors-misconfiguration").asSink() }
7070
}
71+
72+
/**
73+
* A sanitizer for CORS configurations where credentials are explicitly disabled.
74+
* When credentials are false, using "*" for origin is a legitimate pattern.
75+
*/
76+
private class CredentialsDisabledSanitizer extends Sanitizer {
77+
CredentialsDisabledSanitizer() {
78+
exists(DataFlow::SourceNode config, DataFlow::CallNode call |
79+
call.getArgument(0).getALocalSource() = config and
80+
this = config.getAPropertyWrite("origin").getRhs() and
81+
config.getAPropertyWrite("credentials").getRhs().mayHaveBooleanValue(false)
82+
)
83+
}
84+
}
7185
}
Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
/**
22
* @name Permissive CORS configuration
3-
* @description Misconfiguration of CORS HTTP headers allows CSRF attacks.
3+
* @description Cross-origin resource sharing (CORS) policy allows overly broad access.
44
* @kind path-problem
5-
* @problem.severity error
6-
* @security-severity 7.5
5+
* @problem.severity warning
6+
* @security-severity 6.0
77
* @precision high
8-
* @id js/cors-misconfiguration
8+
* @id js/cors-permissive-configuration
99
* @tags security
1010
* external/cwe/cwe-942
1111
*/
@@ -18,5 +18,5 @@ from
1818
CorsQuery::CorsPermissiveConfigurationFlow::PathNode source,
1919
CorsQuery::CorsPermissiveConfigurationFlow::PathNode sink
2020
where CorsQuery::CorsPermissiveConfigurationFlow::flowPath(source, sink)
21-
select sink.getNode(), source, sink, "CORS Origin misconfiguration due to a $@.", source.getNode(),
22-
"too permissive or user controlled value"
21+
select sink.getNode(), source, sink, "CORS Origin allows broad access due to $@.", source.getNode(),
22+
"permissive or user controlled value"

javascript/ql/test/query-tests/Security/CWE-942/CorsPermissiveConfiguration.expected

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
#select
2-
| 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 |
3-
| 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 |
4-
| 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 |
5-
| 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 |
6-
| 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 |
7-
| 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 |
8-
| 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 |
2+
| 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 |
3+
| 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 |
4+
| 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 |
5+
| 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 |
6+
| 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 |
7+
| 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 |
8+
| 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 |
9+
| 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 |
910
edges
1011
| apollo-test.js:8:9:8:59 | user_origin | apollo-test.js:26:25:26:35 | user_origin | provenance | |
1112
| apollo-test.js:8:9:8:59 | user_origin | apollo-test.js:26:25:26:35 | user_origin | provenance | |
@@ -39,4 +40,5 @@ nodes
3940
| express-test.js:26:17:26:19 | '*' | semmle.label | '*' |
4041
| express-test.js:33:17:33:27 | user_origin | semmle.label | user_origin |
4142
| express-test.js:33:17:33:27 | user_origin | semmle.label | user_origin |
43+
| express-test.js:48:17:48:19 | '*' | semmle.label | '*' |
4244
subpaths

javascript/ql/test/query-tests/Security/CWE-942/express-test.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,20 @@ server.on('request', function (req, res) {
3333
origin: user_origin // $ Alert
3434
};
3535
app4.use(cors(corsOption4));
36+
37+
// GOOD: CORS allows any origin but credentials are disabled (safe pattern)
38+
var app5 = express();
39+
var corsOption5 = {
40+
origin: '*',
41+
credentials: false
42+
};
43+
app5.use(cors(corsOption5));
44+
45+
// BAD: CORS allows any origin with credentials enabled
46+
var app6 = express();
47+
var corsOption6 = {
48+
origin: '*', // $ Alert
49+
credentials: true
50+
};
51+
app6.use(cors(corsOption6));
3652
});

0 commit comments

Comments
 (0)