Skip to content

Commit 8c34b7e

Browse files
NapalysNapalys
authored
Merge pull request #20146 from Napalys/js/move-cors-query-from-experimental
JS: Move cors-misconfiguration query from experimental to Security
2 parents 66379de + b2feaac commit 8c34b7e

25 files changed

+287
-379
lines changed

‎javascript/ql/integration-tests/query-suite/javascript-code-scanning.qls.expected‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,5 +83,6 @@ ql/javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql
8383
ql/javascript/ql/src/Security/CWE-915/PrototypePollutingMergeCall.ql
8484
ql/javascript/ql/src/Security/CWE-916/InsufficientPasswordHash.ql
8585
ql/javascript/ql/src/Security/CWE-918/RequestForgery.ql
86+
ql/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.ql
8687
ql/javascript/ql/src/Summary/LinesOfCode.ql
8788
ql/javascript/ql/src/Summary/LinesOfUserCode.ql

‎javascript/ql/integration-tests/query-suite/javascript-security-and-quality.qls.expected‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ ql/javascript/ql/src/Security/CWE-915/PrototypePollutingMergeCall.ql
184184
ql/javascript/ql/src/Security/CWE-916/InsufficientPasswordHash.ql
185185
ql/javascript/ql/src/Security/CWE-918/ClientSideRequestForgery.ql
186186
ql/javascript/ql/src/Security/CWE-918/RequestForgery.ql
187+
ql/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.ql
187188
ql/javascript/ql/src/Statements/DanglingElse.ql
188189
ql/javascript/ql/src/Statements/IgnoreArrayResult.ql
189190
ql/javascript/ql/src/Statements/InconsistentLoopOrientation.ql

‎javascript/ql/integration-tests/query-suite/javascript-security-extended.qls.expected‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,5 +99,6 @@ ql/javascript/ql/src/Security/CWE-915/PrototypePollutingMergeCall.ql
9999
ql/javascript/ql/src/Security/CWE-916/InsufficientPasswordHash.ql
100100
ql/javascript/ql/src/Security/CWE-918/ClientSideRequestForgery.ql
101101
ql/javascript/ql/src/Security/CWE-918/RequestForgery.ql
102+
ql/javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.ql
102103
ql/javascript/ql/src/Summary/LinesOfCode.ql
103104
ql/javascript/ql/src/Summary/LinesOfUserCode.ql

‎javascript/ql/integration-tests/query-suite/not_included_in_qls.expected‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ ql/javascript/ql/src/experimental/Security/CWE-347/decodeJwtWithoutVerificationL
7575
ql/javascript/ql/src/experimental/Security/CWE-444/InsecureHttpParser.ql
7676
ql/javascript/ql/src/experimental/Security/CWE-522-DecompressionBombs/DecompressionBombs.ql
7777
ql/javascript/ql/src/experimental/Security/CWE-918/SSRF.ql
78-
ql/javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfiguration.ql
7978
ql/javascript/ql/src/experimental/StandardLibrary/MultipleArgumentsToSetConstructor.ql
8079
ql/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-020/UntrustedDataToExternalAPI.ql
8180
ql/javascript/ql/src/experimental/heuristics/ql/src/Security/CWE-078/CommandInjection.ql

‎javascript/ql/lib/ext/apollo-server.model.yml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ extensions:
55
data:
66
- ["@apollo/server", "Member[ApolloServer,ApolloServerBase].Argument[0].AnyMember.AnyMember.AnyMember.Parameter[1]", "remote"]
77

8+
- addsTo:
9+
pack: codeql/javascript-all
10+
extensible: sinkModel
11+
data:
12+
- ["@apollo/server", "Member[gql].Argument[0]", "sql-injection"]
13+
- ["@apollo/server", "Member[ApolloServer,ApolloServerBase].Argument[0].Member[cors].Member[origin]", "cors-origin"]
14+
815
- addsTo:
916
pack: codeql/javascript-all
1017
extensible: typeModel
@@ -13,3 +20,9 @@ extensions:
1320
- ["@apollo/server", "apollo-server-express", ""]
1421
- ["@apollo/server", "apollo-server-core", ""]
1522
- ["@apollo/server", "apollo-server", ""]
23+
- ["@apollo/server", "@apollo/apollo-server-express", ""]
24+
- ["@apollo/server", "apollo-server-express", ""]
25+
- ["@apollo/server", "@apollo/server", ""]
26+
- ["@apollo/server", "@apollo/apollo-server-core", ""]
27+
- ["ApolloServer", "@apollo/server", "Member[ApolloServer]"]
28+
- ["GraphQLApollo", "@apollo/server", "Member[gql]"]

‎javascript/ql/lib/ext/cors.model.yml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/javascript-all
4+
extensible: sinkModel
5+
data:
6+
- ["cors", "Argument[0].Member[origin]", "cors-origin"]

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

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* Provides default sources, sinks and sanitizers for reasoning about
3+
* overly permissive CORS configurations, as well as
4+
* extension points for adding your own.
5+
*/
6+
7+
import javascript
8+
9+
/** Module containing sources, sinks, and sanitizers for overly permissive CORS configurations. */
10+
module CorsPermissiveConfiguration {
11+
private newtype TFlowState =
12+
TTaint() or
13+
TPermissive()
14+
15+
/** A flow state to associate with a tracked value. */
16+
class FlowState extends TFlowState {
17+
/** Gets a string representation of this flow state. */
18+
string toString() {
19+
this = TTaint() and result = "taint"
20+
or
21+
this = TPermissive() and result = "permissive"
22+
}
23+
}
24+
25+
/** Predicates for working with flow states. */
26+
module FlowState {
27+
/** A tainted value. */
28+
FlowState taint() { result = TTaint() }
29+
30+
/** A permissive value (true, null, or "*"). */
31+
FlowState permissive() { result = TPermissive() }
32+
}
33+
34+
/**
35+
* A data flow source for permissive CORS configuration.
36+
*/
37+
abstract class Source extends DataFlow::Node { }
38+
39+
/**
40+
* A data flow sink for permissive CORS configuration.
41+
*/
42+
abstract class Sink extends DataFlow::Node { }
43+
44+
/**
45+
* A sanitizer for permissive CORS configuration.
46+
*/
47+
abstract class Sanitizer extends DataFlow::Node { }
48+
49+
/**
50+
* An active threat-model source, considered as a flow source.
51+
*/
52+
private class ActiveThreatModelSourceAsSource extends Source instanceof ActiveThreatModelSource {
53+
ActiveThreatModelSourceAsSource() { not this instanceof ClientSideRemoteFlowSource }
54+
}
55+
56+
/** An overly permissive value for `origin` configuration. */
57+
class PermissiveValue extends Source {
58+
PermissiveValue() {
59+
this.mayHaveBooleanValue(true) or
60+
this.asExpr() instanceof NullLiteral or
61+
this.mayHaveStringValue("*")
62+
}
63+
}
64+
65+
/**
66+
* The value of cors origin when initializing the application.
67+
*/
68+
class CorsOriginSink extends Sink, DataFlow::ValueNode {
69+
CorsOriginSink() { this = ModelOutput::getASinkNode("cors-origin").asSink() }
70+
}
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+
}
85+
}

‎javascript/ql/lib/semmle/javascript/security/CorsPermissiveConfigurationQuery.qll‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* Provides a dataflow taint tracking configuration for reasoning
3+
* about overly permissive CORS configurations.
4+
*
5+
* Note, for performance reasons: only import this file if
6+
* `CorsPermissiveConfiguration::Configuration` is needed,
7+
* otherwise `CorsPermissiveConfigurationCustomizations` should
8+
* be imported instead.
9+
*/
10+
11+
import javascript
12+
import CorsPermissiveConfigurationCustomizations::CorsPermissiveConfiguration
13+
private import CorsPermissiveConfigurationCustomizations::CorsPermissiveConfiguration as CorsPermissiveConfiguration
14+
15+
/**
16+
* A data flow configuration for overly permissive CORS configuration.
17+
*/
18+
module CorsPermissiveConfigurationConfig implements DataFlow::StateConfigSig {
19+
class FlowState = CorsPermissiveConfiguration::FlowState;
20+
21+
predicate isSource(DataFlow::Node source, FlowState state) {
22+
source instanceof PermissiveValue and state = FlowState::permissive()
23+
or
24+
source instanceof RemoteFlowSource and state = FlowState::taint()
25+
}
26+
27+
predicate isSink(DataFlow::Node sink, FlowState state) {
28+
sink instanceof CorsOriginSink and
29+
state = [FlowState::taint(), FlowState::permissive()]
30+
}
31+
32+
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
33+
34+
predicate observeDiffInformedIncrementalMode() { any() }
35+
}
36+
37+
module CorsPermissiveConfigurationFlow =
38+
TaintTracking::GlobalWithState<CorsPermissiveConfigurationConfig>;

‎javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.qhelp‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
9+
A server can use CORS (Cross-Origin Resource Sharing) to relax the
10+
restrictions imposed by the Same-Origin Policy, allowing controlled, secure
11+
cross-origin requests when necessary.
12+
13+
</p>
14+
<p>
15+
16+
A server with an overly permissive CORS configuration may inadvertently
17+
expose sensitive data or enable CSRF attacks, which allow attackers to trick
18+
users into performing unwanted operations on websites they're authenticated to.
19+
20+
</p>
21+
</overview>
22+
23+
<recommendation>
24+
<p>
25+
26+
When the <code>origin</code> is set to <code>true</code>, the server
27+
accepts requests from any origin, potentially exposing the system to
28+
CSRF attacks. Use <code>false</code> as the origin value or implement a whitelist
29+
of allowed origins instead.
30+
31+
</p>
32+
<p>
33+
34+
When the <code>origin</code> is set to <code>null</code>, it can be
35+
exploited by an attacker who can deceive a user into making
36+
requests from a <code>null</code> origin, often hosted within a sandboxed iframe.
37+
38+
</p>
39+
<p>
40+
41+
If the <code>origin</code> value is user-controlled, ensure that the data
42+
is properly sanitized and validated against a whitelist of allowed origins.
43+
44+
</p>
45+
</recommendation>
46+
47+
<example>
48+
<p>
49+
50+
In the following example, <code>server_1</code> accepts requests from any origin
51+
because the value of <code>origin</code> is set to <code>true</code>.
52+
<code>server_2</code> uses user-controlled data for the origin without validation.
53+
54+
</p>
55+
56+
<sample src="examples/CorsPermissiveConfigurationBad.js"/>
57+
58+
<p>
59+
60+
To fix these issues, <code>server_1</code> uses a restrictive CORS configuration
61+
that is not vulnerable to CSRF attacks. <code>server_2</code> properly validates
62+
user-controlled data against a whitelist before using it.
63+
64+
</p>
65+
66+
<sample src="examples/CorsPermissiveConfigurationGood.js"/>
67+
</example>
68+
69+
<references>
70+
<li>Mozilla Developer Network: <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin">CORS, Access-Control-Allow-Origin</a>.</li>
71+
<li>W3C: <a href="https://w3c.github.io/webappsec-cors-for-developers/#resources">CORS for developers, Advice for Resource Owners</a>.</li>
72+
</references>
73+
</qhelp>

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/**
2+
* @name Permissive CORS configuration
3+
* @description Cross-origin resource sharing (CORS) policy allows overly broad access.
4+
* @kind path-problem
5+
* @problem.severity warning
6+
* @security-severity 6.0
7+
* @precision high
8+
* @id js/cors-permissive-configuration
9+
* @tags security
10+
* external/cwe/cwe-942
11+
*/
12+
13+
import javascript
14+
import semmle.javascript.security.CorsPermissiveConfigurationQuery as CorsQuery
15+
import CorsQuery::CorsPermissiveConfigurationFlow::PathGraph
16+
17+
from
18+
CorsQuery::CorsPermissiveConfigurationFlow::PathNode source,
19+
CorsQuery::CorsPermissiveConfigurationFlow::PathNode sink
20+
where CorsQuery::CorsPermissiveConfigurationFlow::flowPath(source, sink)
21+
select sink.getNode(), source, sink, "CORS Origin allows broad access due to $@.", source.getNode(),
22+
"permissive or user controlled value"

0 commit comments

Comments
 (0)