Skip to content

JS: Move cors-misconfiguration query from experimental to Security #20139

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
*/

import javascript
import Cors::Cors
import Apollo::Apollo
private import semmle.javascript.frameworks.Apollo
private import semmle.javascript.frameworks.Cors

/** Module containing sources, sinks, and sanitizers for overly permissive CORS configurations. */
module CorsPermissiveConfiguration {
Expand All @@ -26,6 +26,7 @@ module CorsPermissiveConfiguration {
this = TWildcard() and result = "wildcard"
}

/** DEPRECATED: Converts this flow state to a flow label. */
deprecated DataFlow::FlowLabel toFlowLabel() {
this = TTaint() and result.isTaint()
or
Expand All @@ -37,6 +38,7 @@ module CorsPermissiveConfiguration {

/** Predicates for working with flow states. */
module FlowState {
/** DEPRECATED: Gets a flow state from a flow label. */
deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label }

/** A tainted value. */
Expand Down Expand Up @@ -81,13 +83,15 @@ module CorsPermissiveConfiguration {
TrueAndNull() { this = "TrueAndNull" }
}

/** DEPRECATED: Gets a flow label representing `true` and `null` values. */
deprecated TrueAndNull truenullLabel() { any() }

/** A flow label representing `*` value. */
abstract deprecated class Wildcard extends DataFlow::FlowLabel {
Wildcard() { this = "Wildcard" }
}

/** DEPRECATED: Gets a flow label representing `*` value. */
deprecated Wildcard wildcardLabel() { any() }

/** An overly permissive value for `origin` (Apollo) */
Expand All @@ -105,7 +109,7 @@ module CorsPermissiveConfiguration {
*/
class CorsApolloServer extends Sink, DataFlow::ValueNode {
CorsApolloServer() {
exists(ApolloServer agql |
exists(Apollo::ApolloServer agql |
this =
agql.getOptionArgument(0, "cors").getALocalSource().getAPropertyWrite("origin").getRhs()
)
Expand All @@ -125,7 +129,7 @@ module CorsPermissiveConfiguration {
* An express route setup configured with the `cors` package.
*/
class CorsConfiguration extends DataFlow::MethodCallNode {
Cors corsConfig;
Cors::Cors corsConfig;

CorsConfiguration() {
exists(Express::RouteSetup setup | this = setup |
Expand All @@ -136,6 +140,6 @@ module CorsPermissiveConfiguration {
}

/** Gets the expression that configures `cors` on this route setup. */
Cors getCorsConfiguration() { result = corsConfig }
Cors::Cors getCorsConfiguration() { result = corsConfig }
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* @name overly CORS configuration
* @name Permissive CORS configuration
* @description Misconfiguration of CORS HTTP headers allows CSRF attacks.
* @kind path-problem
* @problem.severity error
Expand All @@ -11,11 +11,12 @@
*/

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

from
CorsPermissiveConfigurationFlow::PathNode source, CorsPermissiveConfigurationFlow::PathNode sink
where CorsPermissiveConfigurationFlow::flowPath(source, sink)
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"

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
query: Security/CWE-942/CorsPermissiveConfiguration.ql
postprocess: utils/test/InlineExpectationsTestQuery.ql
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ var https = require('https'),
var server = https.createServer(function () { });

server.on('request', function (req, res) {
let user_origin = url.parse(req.url, true).query.origin;
let user_origin = url.parse(req.url, true).query.origin; // $ Source
// BAD: CORS too permissive
const server_1 = new ApolloServer({
cors: { origin: true }
cors: { origin: true } // $ Alert
});

// GOOD: restrictive CORS
Expand All @@ -18,11 +18,11 @@ server.on('request', function (req, res) {

// BAD: CORS too permissive
const server_3 = new ApolloServer({
cors: { origin: null }
cors: { origin: null } // $ Alert
});

// BAD: CORS is controlled by user
const server_4 = new ApolloServer({
cors: { origin: user_origin }
cors: { origin: user_origin } // $ Alert
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ var https = require('https'),
var server = https.createServer(function () { });

server.on('request', function (req, res) {
let user_origin = url.parse(req.url, true).query.origin;
let user_origin = url.parse(req.url, true).query.origin; // $ Source

// BAD: CORS too permissive, default value is *
var app1 = express();
Expand All @@ -23,14 +23,14 @@ server.on('request', function (req, res) {
// BAD: CORS too permissive
var app3 = express();
var corsOption3 = {
origin: '*'
origin: '*' // $ Alert
};
app3.use(cors(corsOption3));

// BAD: CORS is controlled by user
var app4 = express();
var corsOption4 = {
origin: user_origin
origin: user_origin // $ Alert
};
app4.use(cors(corsOption4));
});