Skip to content

Commit 871bc3b

Browse files
committed
JS: Port experimental CorsPermissiveConfiguration to ConfigSig
The tests show a new (source, sink) pair for an already-flagged sink. Not sure why it was not flagged originally since the data flow path seems valid, given the steps provided by our models.
1 parent f5a6485 commit 871bc3b

File tree

3 files changed

+56
-51
lines changed

3 files changed

+56
-51
lines changed

javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfiguration.ql

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@
1212

1313
import javascript
1414
import CorsPermissiveConfigurationQuery
15-
import DataFlow::PathGraph
15+
import CorsPermissiveConfigurationFlow::PathGraph
1616

17-
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
18-
where cfg.hasFlowPath(source, sink)
17+
from
18+
CorsPermissiveConfigurationFlow::PathNode source, CorsPermissiveConfigurationFlow::PathNode sink
19+
where CorsPermissiveConfigurationFlow::flowPath(source, sink)
1920
select sink.getNode(), source, sink, "CORS Origin misconfiguration due to a $@.", source.getNode(),
2021
"too permissive or user controlled value"

javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfigurationQuery.qll

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,46 @@ import CorsPermissiveConfigurationCustomizations::CorsPermissiveConfiguration
1414
/**
1515
* A data flow configuration for overly permissive CORS configuration.
1616
*/
17-
class Configuration extends TaintTracking::Configuration {
18-
Configuration() { this = "CorsPermissiveConfiguration" }
17+
module CorsPermissiveConfigurationConfig implements DataFlow::StateConfigSig {
18+
class FlowState = DataFlow::FlowLabel;
1919

20-
override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) {
20+
predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) {
2121
source instanceof TrueNullValue and label = truenullLabel()
2222
or
2323
source instanceof WildcardValue and label = wildcardLabel()
2424
or
2525
source instanceof RemoteFlowSource and label = DataFlow::FlowLabel::taint()
2626
}
2727

28-
override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
28+
predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
2929
sink instanceof CorsApolloServer and label = [DataFlow::FlowLabel::taint(), truenullLabel()]
3030
or
3131
sink instanceof ExpressCors and label = [DataFlow::FlowLabel::taint(), wildcardLabel()]
3232
}
3333

34+
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
35+
}
36+
37+
module CorsPermissiveConfigurationFlow =
38+
TaintTracking::GlobalWithState<CorsPermissiveConfigurationConfig>;
39+
40+
/**
41+
* DEPRECATED. Use the `CorsPermissiveConfigurationFlow` module instead.
42+
*/
43+
deprecated class Configuration extends TaintTracking::Configuration {
44+
Configuration() { this = "CorsPermissiveConfiguration" }
45+
46+
override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) {
47+
CorsPermissiveConfigurationConfig::isSource(source, label)
48+
}
49+
50+
override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
51+
CorsPermissiveConfigurationConfig::isSink(sink, label)
52+
}
53+
3454
override predicate isSanitizer(DataFlow::Node node) {
3555
super.isSanitizer(node) or
36-
node instanceof Sanitizer
56+
CorsPermissiveConfigurationConfig::isBarrier(node)
3757
}
3858
}
3959

Lines changed: 27 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,34 @@
1-
nodes
2-
| apollo-test.js:8:9:8:59 | user_origin |
3-
| apollo-test.js:8:23:8:46 | url.par ... , true) |
4-
| apollo-test.js:8:23:8:52 | url.par ... ).query |
5-
| apollo-test.js:8:23:8:59 | url.par ... .origin |
6-
| apollo-test.js:8:33:8:39 | req.url |
7-
| apollo-test.js:8:33:8:39 | req.url |
8-
| apollo-test.js:11:25:11:28 | true |
9-
| apollo-test.js:11:25:11:28 | true |
10-
| apollo-test.js:11:25:11:28 | true |
11-
| apollo-test.js:21:25:21:28 | null |
12-
| apollo-test.js:21:25:21:28 | null |
13-
| apollo-test.js:21:25:21:28 | null |
14-
| apollo-test.js:26:25:26:35 | user_origin |
15-
| apollo-test.js:26:25:26:35 | user_origin |
16-
| express-test.js:10:9:10:59 | user_origin |
17-
| express-test.js:10:23:10:46 | url.par ... , true) |
18-
| express-test.js:10:23:10:52 | url.par ... ).query |
19-
| express-test.js:10:23:10:59 | url.par ... .origin |
20-
| express-test.js:10:33:10:39 | req.url |
21-
| express-test.js:10:33:10:39 | req.url |
22-
| express-test.js:26:17:26:19 | '*' |
23-
| express-test.js:26:17:26:19 | '*' |
24-
| express-test.js:26:17:26:19 | '*' |
25-
| express-test.js:33:17:33:27 | user_origin |
26-
| express-test.js:33:17:33:27 | user_origin |
271
edges
28-
| apollo-test.js:8:9:8:59 | user_origin | apollo-test.js:26:25:26:35 | user_origin |
29-
| apollo-test.js:8:9:8:59 | user_origin | apollo-test.js:26:25:26:35 | user_origin |
30-
| apollo-test.js:8:23:8:46 | url.par ... , true) | apollo-test.js:8:23:8:52 | url.par ... ).query |
31-
| apollo-test.js:8:23:8:52 | url.par ... ).query | apollo-test.js:8:23:8:59 | url.par ... .origin |
32-
| apollo-test.js:8:23:8:59 | url.par ... .origin | apollo-test.js:8:9:8:59 | user_origin |
33-
| apollo-test.js:8:33:8:39 | req.url | apollo-test.js:8:23:8:46 | url.par ... , true) |
34-
| apollo-test.js:8:33:8:39 | req.url | apollo-test.js:8:23:8:46 | url.par ... , true) |
35-
| apollo-test.js:11:25:11:28 | true | apollo-test.js:11:25:11:28 | true |
36-
| apollo-test.js:21:25:21:28 | null | apollo-test.js:21:25:21:28 | null |
37-
| express-test.js:10:9:10:59 | user_origin | express-test.js:33:17:33:27 | user_origin |
38-
| express-test.js:10:9:10:59 | user_origin | express-test.js:33:17:33:27 | user_origin |
39-
| express-test.js:10:23:10:46 | url.par ... , true) | express-test.js:10:23:10:52 | url.par ... ).query |
40-
| express-test.js:10:23:10:52 | url.par ... ).query | express-test.js:10:23:10:59 | url.par ... .origin |
41-
| express-test.js:10:23:10:59 | url.par ... .origin | express-test.js:10:9:10:59 | user_origin |
42-
| express-test.js:10:33:10:39 | req.url | express-test.js:10:23:10:46 | url.par ... , true) |
43-
| express-test.js:10:33:10:39 | req.url | express-test.js:10:23:10:46 | url.par ... , true) |
44-
| express-test.js:26:17:26:19 | '*' | express-test.js:26:17:26:19 | '*' |
2+
| apollo-test.js:8:9:8:59 | user_origin | apollo-test.js:26:25:26:35 | user_origin | provenance | |
3+
| apollo-test.js:8:9:8:59 | user_origin | apollo-test.js:26:25:26:35 | user_origin | provenance | |
4+
| apollo-test.js:8:23:8:46 | url.par ... , true) | apollo-test.js:8:9:8:59 | user_origin | provenance | |
5+
| apollo-test.js:8:23:8:46 | url.par ... , true) | apollo-test.js:8:9:8:59 | user_origin | provenance | |
6+
| apollo-test.js:8:33:8:39 | req.url | apollo-test.js:8:23:8:46 | url.par ... , true) | provenance | |
7+
| apollo-test.js:8:42:8:45 | true | apollo-test.js:8:23:8:46 | url.par ... , true) | provenance | |
8+
| express-test.js:10:9:10:59 | user_origin | express-test.js:33:17:33:27 | user_origin | provenance | |
9+
| express-test.js:10:23:10:46 | url.par ... , true) | express-test.js:10:9:10:59 | user_origin | provenance | |
10+
| express-test.js:10:33:10:39 | req.url | express-test.js:10:23:10:46 | url.par ... , true) | provenance | |
11+
nodes
12+
| apollo-test.js:8:9:8:59 | user_origin | semmle.label | user_origin |
13+
| apollo-test.js:8:9:8:59 | user_origin | semmle.label | user_origin |
14+
| apollo-test.js:8:23:8:46 | url.par ... , true) | semmle.label | url.par ... , true) |
15+
| apollo-test.js:8:23:8:46 | url.par ... , true) | semmle.label | url.par ... , true) |
16+
| apollo-test.js:8:33:8:39 | req.url | semmle.label | req.url |
17+
| apollo-test.js:8:42:8:45 | true | semmle.label | true |
18+
| apollo-test.js:11:25:11:28 | true | semmle.label | true |
19+
| apollo-test.js:21:25:21:28 | null | semmle.label | null |
20+
| apollo-test.js:26:25:26:35 | user_origin | semmle.label | user_origin |
21+
| apollo-test.js:26:25:26:35 | user_origin | semmle.label | user_origin |
22+
| express-test.js:10:9:10:59 | user_origin | semmle.label | user_origin |
23+
| express-test.js:10:23:10:46 | url.par ... , true) | semmle.label | url.par ... , true) |
24+
| express-test.js:10:33:10:39 | req.url | semmle.label | req.url |
25+
| express-test.js:26:17:26:19 | '*' | semmle.label | '*' |
26+
| express-test.js:33:17:33:27 | user_origin | semmle.label | user_origin |
27+
subpaths
4528
#select
4629
| 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 |
4730
| 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 |
4831
| 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 |
32+
| 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 |
4933
| 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 |
5034
| 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 |

0 commit comments

Comments
 (0)