Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions javascript/ql/lib/ext/apollo-server.model.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ 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"]
- ["@apollo/server", "Member[ApolloServer,ApolloServerBase].Argument[0].Member[cors].Member[origin]", "cors-origin"]

- addsTo:
pack: codeql/javascript-all
extensible: typeModel
Expand All @@ -13,3 +20,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]"]
6 changes: 6 additions & 0 deletions javascript/ql/lib/ext/cors.model.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
extensions:
- addsTo:
pack: codeql/javascript-all
extensible: sinkModel
data:
- ["cors", "Argument[0].Member[origin]", "cors-origin"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/**
* Provides default sources, sinks and sanitizers for reasoning about
* overly permissive CORS configurations, as well as
* extension points for adding your own.
*/

import javascript

/** Module containing sources, sinks, and sanitizers for overly permissive CORS configurations. */
module CorsPermissiveConfiguration {
private newtype TFlowState =
TTaint() or
TPermissive()

/** 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 = TPermissive() and result = "permissive"
}
}

/** Predicates for working with flow states. */
module FlowState {
/** A tainted value. */

Check warning

Code scanning / CodeQL

Predicate QLDoc style Warning

The QLDoc for a predicate with a result should start with 'Gets'.
FlowState taint() { result = TTaint() }

/** A permissive value (true, null, or "*"). */

Check warning

Code scanning / CodeQL

Predicate QLDoc style Warning

The QLDoc for a predicate with a result should start with 'Gets'.
FlowState permissive() { result = TPermissive() }
}

/**
* A data flow source for permissive CORS configuration.
*/
abstract class Source extends DataFlow::Node { }

/**
* A data flow sink for permissive CORS configuration.
*/
abstract class Sink extends DataFlow::Node { }

/**
* A sanitizer for permissive CORS configuration.
*/
abstract class Sanitizer extends DataFlow::Node { }

/**
* An active threat-model source, considered as a flow source.
*/
private class ActiveThreatModelSourceAsSource extends Source instanceof ActiveThreatModelSource {
ActiveThreatModelSourceAsSource() { not this instanceof ClientSideRemoteFlowSource }
}

/** An overly permissive value for `origin` configuration. */
class PermissiveValue extends Source {
PermissiveValue() {
this.mayHaveBooleanValue(true) or
this.asExpr() instanceof NullLiteral or
this.mayHaveStringValue("*")
}
}

/**
* The value of cors origin when initializing the application.
*/
class CorsOriginSink extends Sink, DataFlow::ValueNode {
CorsOriginSink() { this = ModelOutput::getASinkNode("cors-origin").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)
)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/**
* Provides a dataflow taint tracking configuration for reasoning
* about overly permissive CORS configurations.
*
* Note, for performance reasons: only import this file if
* `CorsPermissiveConfiguration::Configuration` is needed,
* otherwise `CorsPermissiveConfigurationCustomizations` should
* be imported instead.
*/

import javascript
import CorsPermissiveConfigurationCustomizations::CorsPermissiveConfiguration
private import CorsPermissiveConfigurationCustomizations::CorsPermissiveConfiguration as CorsPermissiveConfiguration

/**
* A data flow configuration for overly permissive CORS configuration.
*/
module CorsPermissiveConfigurationConfig implements DataFlow::StateConfigSig {
class FlowState = CorsPermissiveConfiguration::FlowState;

predicate isSource(DataFlow::Node source, FlowState state) {
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::permissive()]
}

predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }

predicate observeDiffInformedIncrementalMode() { any() }
}

module CorsPermissiveConfigurationFlow =
TaintTracking::GlobalWithState<CorsPermissiveConfigurationConfig>;
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>

<overview>
<p>

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.

</p>
<p>

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.

</p>
</overview>

<recommendation>
<p>

When the <code>origin</code> is set to <code>true</code>, the server
accepts requests from any origin, potentially exposing the system to
CSRF attacks. Use <code>false</code> as the origin value or implement a whitelist
of allowed origins instead.

</p>
<p>

When the <code>origin</code> is set to <code>null</code>, it can be
exploited by an attacker who can deceive a user into making
requests from a <code>null</code> origin, often hosted within a sandboxed iframe.

</p>
<p>

If the <code>origin</code> value is user-controlled, ensure that the data
is properly sanitized and validated against a whitelist of allowed origins.

</p>
</recommendation>

<example>
<p>

In the following example, <code>server_1</code> accepts requests from any origin
because the value of <code>origin</code> is set to <code>true</code>.
<code>server_2</code> uses user-controlled data for the origin without validation.

</p>

<sample src="examples/CorsPermissiveConfigurationBad.js"/>

<p>

To fix these issues, <code>server_1</code> uses a restrictive CORS configuration
that is not vulnerable to CSRF attacks. <code>server_2</code> properly validates
user-controlled data against a whitelist before using it.

</p>

<sample src="examples/CorsPermissiveConfigurationGood.js"/>
</example>

<references>
<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>
<li>W3C: <a href="https://w3c.github.io/webappsec-cors-for-developers/#resources">CORS for developers, Advice for Resource Owners</a>.</li>
</references>
</qhelp>
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* @name Permissive CORS configuration
* @description Cross-origin resource sharing (CORS) policy allows overly broad access.
* @kind path-problem
* @problem.severity warning
* @security-severity 6.0
* @precision high
* @id js/cors-permissive-configuration
* @tags security
* external/cwe/cwe-942
*/

import javascript
import semmle.javascript.security.CorsPermissiveConfigurationQuery as CorsQuery
import CorsQuery::CorsPermissiveConfigurationFlow::PathGraph

from
CorsQuery::CorsPermissiveConfigurationFlow::PathNode source,
CorsQuery::CorsPermissiveConfigurationFlow::PathNode sink
where CorsQuery::CorsPermissiveConfigurationFlow::flowPath(source, sink)
select sink.getNode(), source, sink, "CORS Origin allows broad access due to $@.", source.getNode(),
"permissive or user controlled value"
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The query "Permissive CORS configuration" (`js/cors-permissive-configuration`) has been promoted from experimental and is now part of the default security suite.
36 changes: 0 additions & 36 deletions javascript/ql/src/experimental/Security/CWE-942/Apollo.qll

This file was deleted.

24 changes: 0 additions & 24 deletions javascript/ql/src/experimental/Security/CWE-942/Cors.qll

This file was deleted.

Loading
Loading