JS: Add Permissive CORS query (CWE-942) #14342
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| /** | ||
| * Provides classes for working with Apollo GraphQL connectors. | ||
| */ | ||
|
|
||
| import javascript | ||
|
|
||
| /** Provides classes modeling concepts of Apollo GraphQL. */ | ||
| module ApolloGraphQL { | ||
| /** A string-valued expression that is interpreted as a Apollo GraphQL query. */ | ||
| abstract class GraphQLString extends DataFlow::Node { } | ||
|
|
||
| /** A string-valued expression that is interpreted as a Apollo GraphQL query. */ | ||
| abstract class ApolloGraphQLServer extends DataFlow::Node { } | ||
|
maikypedia marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| /** | ||
| * Provides classes modeling the apollo packages [@apollo/server](https://npmjs.com/package/@apollo/server`) | ||
| */ | ||
| private module Apollo { | ||
|
maikypedia marked this conversation as resolved.
Outdated
|
||
| /** Get an instanceof of `Apollo` */ | ||
| private API::Node apollo() { | ||
| result = | ||
| API::moduleImport([ | ||
| "@apollo/server", "apollo/server", "@apollo/apollo-server-express", | ||
|
maikypedia marked this conversation as resolved.
Outdated
|
||
| "@apollo/apollo-server-core", "apollo-server", "apollo-server-express" | ||
| ]).getMember("ApolloServer") | ||
| } | ||
|
|
||
| /** Get an instanceof of `gql` */ | ||
| private API::Node gql() { | ||
|
maikypedia marked this conversation as resolved.
Outdated
|
||
| result = | ||
| API::moduleImport([ | ||
| "@apollo/server", "apollo/server", "@apollo/apollo-server-express", | ||
| "@apollo/apollo-server-core", "apollo-server", "apollo-server-express" | ||
|
maikypedia marked this conversation as resolved.
Outdated
|
||
| ]).getMember("gql") | ||
| } | ||
|
|
||
| /** A string that is interpreted as a GraphQL query by a `octokit` package. */ | ||
| private class ApolloGraphQLString extends GraphQL::GraphQLString { | ||
| ApolloGraphQLString() { this = gql().getACall() } | ||
| } | ||
|
Contributor
There was a problem hiding this comment.
In this case the Also, a test of this would be nice.
Contributor
Author
There was a problem hiding this comment. Hi Erik 😄 ! That part is not finished because I wanted to ask how it could be done because I couldn't get it to work. I don't know why using const { execFile } = require("child_process");
const { ApolloServer, gql } = require("apollo-server");
execFile("whoami");
const typeDefs = gql`
type Query {
hello: String
}
`;
Contributor
There was a problem hiding this comment. Yes, tagged template strings are not currently seen as calls in our analysis, and you're not the only one that's noticed this limitation recently. Your use with API-graphs should just magically work after tagged template strings are added as I'll look into fixing that limitation of our analysis later, but it might take a while before I get started on that. If you're feeling adventurous you can try it yourself (that would definitely belong in a separate PR).
Contributor
There was a problem hiding this comment. I just opened a PR to recognize tagged template literals as a I suggest you take a look at that PR and incorporate it into your PR after my PR has been merged.
Contributor
Author
There was a problem hiding this comment. Sure! Thanks :) 👍
Contributor
There was a problem hiding this comment. #14405 has been merged, so you can incorporate that into this PR. |
||
|
|
||
| /** A string that is interpreted as a GraphQL query by a `graphql` package. */ | ||
| private class ApolloServer extends ApolloGraphQL::ApolloGraphQLServer { | ||
|
maikypedia marked this conversation as resolved.
Outdated
|
||
| ApolloServer() { this = apollo().getAnInstantiation() } | ||
|
|
||
| predicate isPermissive() { | ||
|
Contributor
There was a problem hiding this comment. This predicate is never used. |
||
| this.(DataFlow::NewNode) | ||
| .getOptionArgument(0, "cors") | ||
| .getALocalSource() | ||
| .getAPropertyWrite("origin") | ||
| .getRhs() | ||
| .mayHaveBooleanValue(true) | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,50 @@ | ||||||
| /** | ||||||
| * 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 CorsPermissiveConfiguration { | ||||||
|
maikypedia marked this conversation as resolved.
Outdated
|
||||||
| /** | ||||||
| * 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 { } | ||||||
|
|
||||||
| /** A source of remote user input, considered as a flow source for CORS misconfiguration. */ | ||||||
| class RemoteFlowSourceAsSource extends Source instanceof RemoteFlowSource { | ||||||
| RemoteFlowSourceAsSource() { not this instanceof ClientSideRemoteFlowSource } | ||||||
| } | ||||||
|
|
||||||
| /** true and null are considered bad values */ | ||||||
|
|
||||||
| class BadValues extends Source instanceof DataFlow::Node { | ||||||
|
Contributor
There was a problem hiding this comment.
Suggested change
Adding |
||||||
| BadValues() { this.mayHaveBooleanValue(true) or this.asExpr() instanceof NullLiteral } | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * The value of cors origin when initializing the application. | ||||||
| */ | ||||||
| class CorsApolloServer extends Sink, DataFlow::ValueNode { | ||||||
| CorsApolloServer() { | ||||||
| exists(ApolloGraphQL::ApolloGraphQLServer agql | | ||||||
| this = | ||||||
| agql.(DataFlow::NewNode) | ||||||
| .getOptionArgument(0, "cors") | ||||||
| .getALocalSource() | ||||||
| .getAPropertyWrite("origin") | ||||||
| .getRhs() | ||||||
| ) | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| /** | ||
| * 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 | ||
|
|
||
| /** | ||
| * A data flow configuration for overly permissive CORS configuration. | ||
| */ | ||
| class Configuration extends TaintTracking::Configuration { | ||
| Configuration() { this = "CorsPermissiveConfiguration" } | ||
|
|
||
| override predicate isSource(DataFlow::Node source) { source instanceof Source } | ||
|
|
||
| override predicate isSink(DataFlow::Node sink) { sink instanceof Sink } | ||
|
|
||
| override predicate isSanitizer(DataFlow::Node node) { | ||
| super.isSanitizer(node) or | ||
| node instanceof Sanitizer | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| /** | ||
|
maikypedia marked this conversation as resolved.
Outdated
|
||
| * @name overly CORS configuration | ||
| * @description Misconfiguration of CORS HTTP headers allows CSRF attacks. | ||
| * @kind path-problem | ||
| * @problem.severity error | ||
| * @security-severity 7.5 | ||
| * @precision high | ||
| * @id js/cors-misconfiguration | ||
| * @tags security | ||
| * external/cwe/cwe-942 | ||
| */ | ||
|
|
||
| import javascript | ||
| import semmle.javascript.security.dataflow.CorsPermissiveConfigurationQuery | ||
| import DataFlow::PathGraph | ||
|
|
||
| from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink | ||
| where cfg.hasFlowPath(source, sink) | ||
| select sink.getNode(), source, sink, "$@ misconfiguration due to a $@.", sink.getNode(), | ||
|
|
||
| "CORS Origin", source.getNode(), "too permissive or user controlled value" | ||
|
maikypedia marked this conversation as resolved.
Outdated
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| nodes | ||
| | tst.js:8:9:8:59 | user_origin | | ||
| | tst.js:8:23:8:46 | url.par ... , true) | | ||
| | tst.js:8:23:8:52 | url.par ... ).query | | ||
| | tst.js:8:23:8:59 | url.par ... .origin | | ||
| | tst.js:8:33:8:39 | req.url | | ||
| | tst.js:8:33:8:39 | req.url | | ||
| | tst.js:8:42:8:45 | true | | ||
| | tst.js:8:42:8:45 | true | | ||
| | tst.js:11:25:11:28 | true | | ||
| | tst.js:11:25:11:28 | true | | ||
| | tst.js:11:25:11:28 | true | | ||
| | tst.js:16:25:16:28 | true | | ||
| | tst.js:16:25:16:28 | true | | ||
| | tst.js:16:25:16:28 | true | | ||
| | tst.js:26:25:26:28 | null | | ||
| | tst.js:26:25:26:28 | null | | ||
| | tst.js:26:25:26:28 | null | | ||
| | tst.js:31:25:31:35 | user_origin | | ||
| | tst.js:31:25:31:35 | user_origin | | ||
| edges | ||
| | tst.js:8:9:8:59 | user_origin | tst.js:31:25:31:35 | user_origin | | ||
| | tst.js:8:9:8:59 | user_origin | tst.js:31:25:31:35 | user_origin | | ||
| | tst.js:8:23:8:46 | url.par ... , true) | tst.js:8:23:8:52 | url.par ... ).query | | ||
| | tst.js:8:23:8:52 | url.par ... ).query | tst.js:8:23:8:59 | url.par ... .origin | | ||
| | tst.js:8:23:8:59 | url.par ... .origin | tst.js:8:9:8:59 | user_origin | | ||
| | tst.js:8:33:8:39 | req.url | tst.js:8:23:8:46 | url.par ... , true) | | ||
| | tst.js:8:33:8:39 | req.url | tst.js:8:23:8:46 | url.par ... , true) | | ||
| | tst.js:8:42:8:45 | true | tst.js:8:23:8:46 | url.par ... , true) | | ||
| | tst.js:8:42:8:45 | true | tst.js:8:23:8:46 | url.par ... , true) | | ||
| | tst.js:11:25:11:28 | true | tst.js:11:25:11:28 | true | | ||
| | tst.js:16:25:16:28 | true | tst.js:16:25:16:28 | true | | ||
| | tst.js:26:25:26:28 | null | tst.js:26:25:26:28 | null | | ||
| #select | ||
| | tst.js:11:25:11:28 | true | tst.js:11:25:11:28 | true | tst.js:11:25:11:28 | true | $@ misconfiguration due to a $@. | tst.js:11:25:11:28 | true | CORS Origin | tst.js:11:25:11:28 | true | too permissive or user controlled value | | ||
| | tst.js:16:25:16:28 | true | tst.js:16:25:16:28 | true | tst.js:16:25:16:28 | true | $@ misconfiguration due to a $@. | tst.js:16:25:16:28 | true | CORS Origin | tst.js:16:25:16:28 | true | too permissive or user controlled value | | ||
| | tst.js:26:25:26:28 | null | tst.js:26:25:26:28 | null | tst.js:26:25:26:28 | null | $@ misconfiguration due to a $@. | tst.js:26:25:26:28 | null | CORS Origin | tst.js:26:25:26:28 | null | too permissive or user controlled value | | ||
| | tst.js:31:25:31:35 | user_origin | tst.js:8:33:8:39 | req.url | tst.js:31:25:31:35 | user_origin | $@ misconfiguration due to a $@. | tst.js:31:25:31:35 | user_origin | CORS Origin | tst.js:8:33:8:39 | req.url | too permissive or user controlled value | | ||
| | tst.js:31:25:31:35 | user_origin | tst.js:8:42:8:45 | true | tst.js:31:25:31:35 | user_origin | $@ misconfiguration due to a $@. | tst.js:31:25:31:35 | user_origin | CORS Origin | tst.js:8:42:8:45 | true | too permissive or user controlled value | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Security/CWE-942/CorsPermissiveConfiguration.ql |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import { ApolloServer } from 'apollo-server'; | ||
| var https = require('https'), | ||
| url = require('url'); | ||
|
|
||
| var server = https.createServer(function () { }); | ||
|
|
||
| server.on('request', function (req, res) { | ||
| let user_origin = url.parse(req.url, true).query.origin; | ||
| // BAD: attacker can choose the value of origin | ||
| const server_1 = new ApolloServer({ | ||
| cors: { origin: true } | ||
| }); | ||
|
|
||
| // BAD: CORS too permissive | ||
| const server_2 = new ApolloServer({ | ||
| cors: { origin: true } | ||
| }); | ||
|
|
||
| // GOOD: restrictive CORS | ||
| const server_3 = new ApolloServer({ | ||
| cors: false | ||
| }); | ||
|
|
||
| // BAD: CORS too permissive | ||
| const server_4 = new ApolloServer({ | ||
| cors: { origin: null } | ||
| }); | ||
|
|
||
| // BAD: CORS is controlled by user | ||
| const server_5 = new ApolloServer({ | ||
| cors: { origin: user_origin } | ||
| }); | ||
| }); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.