Skip to content

Commit 816eebb

Browse files
maikypediamaikypedia
committed
Add .qhelp and apply some review changes
1 parent 142ab01 commit 816eebb

File tree

13 files changed

+189
-116
lines changed

13 files changed

+189
-116
lines changed

‎javascript/ql/lib/javascript.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ import semmle.javascript.frameworks.ActionsLib
7171
import semmle.javascript.frameworks.Angular2
7272
import semmle.javascript.frameworks.AngularJS
7373
import semmle.javascript.frameworks.Anser
74-
import semmle.javascript.frameworks.ApolloGraphQL
74+
import semmle.javascript.frameworks.Apollo
7575
import semmle.javascript.frameworks.AsyncPackage
7676
import semmle.javascript.frameworks.AWS
7777
import semmle.javascript.frameworks.Azure

‎javascript/ql/lib/semmle/javascript/frameworks/Apollo.qll

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/**
2+
* Provides classes for working with Apollo GraphQL connectors.
3+
*/
4+
5+
import javascript
6+
7+
/** Provides classes modeling the apollo packages [@apollo/server](https://npmjs.com/package/@apollo/server`) */
8+
module Apollo {
9+
/** Get an instanceof of `Apollo` */
10+
private API::Node apollo() {
11+
result =
12+
API::moduleImport([
13+
"@apollo/server", "@apollo/apollo-server-express", "@apollo/apollo-server-core",
14+
"apollo-server", "apollo-server-express"
15+
]).getMember("ApolloServer")
16+
}
17+
18+
/** Get an instanceof of the `gql` function that parses GraphQL strings. */
19+
private API::Node gql() {
20+
result =
21+
API::moduleImport([
22+
"@apollo/server", "@apollo/apollo-server-express", "@apollo/apollo-server-core",
23+
"apollo-server", "apollo-server-express"
24+
]).getMember("gql")
25+
}
26+
27+
/** A string that is interpreted as a GraphQL query by a `graphql` package. */
28+
class ApolloServer extends API::NewNode {
29+
ApolloServer() { this = apollo().getAnInstantiation() }
30+
}
31+
32+
/** A string that is interpreted as a GraphQL query by a `apollo` package. */
33+
class ApolloGraphQLString extends GraphQL::GraphQLString {
34+
ApolloGraphQLString() { this = gql().getACall().getArgument(0) }
35+
}
36+
}

‎javascript/ql/lib/semmle/javascript/frameworks/ApolloGraphQL.qll

Lines changed: 0 additions & 56 deletions
This file was deleted.

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ module CorsPermissiveConfiguration {
2727
RemoteFlowSourceAsSource() { not this instanceof ClientSideRemoteFlowSource }
2828
}
2929

30-
/** true and null are considered bad values */
31-
class BadValues extends Source instanceof DataFlow::Node {
30+
/** An overfly permissive value for `origin` */
31+
class BadValues extends Source {
3232
BadValues() { this.mayHaveBooleanValue(true) or this.asExpr() instanceof NullLiteral }
3333
}
3434

@@ -37,13 +37,9 @@ module CorsPermissiveConfiguration {
3737
*/
3838
class CorsApolloServer extends Sink, DataFlow::ValueNode {
3939
CorsApolloServer() {
40-
exists(ApolloGraphQL::ApolloGraphQLServer agql |
40+
exists(Apollo::ApolloServer agql |
4141
this =
42-
agql.(DataFlow::NewNode)
43-
.getOptionArgument(0, "cors")
44-
.getALocalSource()
45-
.getAPropertyWrite("origin")
46-
.getRhs()
42+
agql.getOptionArgument(0, "cors").getALocalSource().getAPropertyWrite("origin").getRhs()
4743
)
4844
}
4945
}

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
9+
A server can use <code>CORS</code> (Cross-Origin Resource Sharing) to relax the
10+
restrictions imposed by the <code>SOP</code> (Same-Origin Policy), allowing controlled, secure
11+
cross-origin requests when necessary.
12+
13+
A server with an overly permissive <code>CORS</code> configuration may inadvertently
14+
expose sensitive data or lead to <code>CSRF</code> which is an attack that allows attackers to trick
15+
users into performing unwanted operations in websites they're authenticated to.
16+
17+
</p>
18+
19+
</overview>
20+
21+
<recommendation>
22+
<p>
23+
24+
When the <code>origin</code> is set to <code>true</code>, it signifies that the server
25+
is accepting requests from <code>any</code> origin, potentially exposing the system to
26+
CSRF attacks. This can be fixed using <code>false</code> as origin value or using a whitelist.
27+
28+
</p>
29+
<p>
30+
31+
On the other hand, if the <code>origin</code> is
32+
set to <code>null</code>, it can be exploited by an attacker to deceive a user into making
33+
requests from a <code>null</code> origin form, often hosted within a sandboxed iframe.
34+
35+
</p>
36+
37+
<p>
38+
39+
If the <code>origin</code> value is user controlled, make sure that the data
40+
is properly sanitized.
41+
42+
</p>
43+
</recommendation>
44+
45+
<example>
46+
<p>
47+
48+
In the example below, the <code>server_1</code> accepts requests from any origin
49+
since the value of <code>origin</code> is set to <code>true</code>.
50+
And <code>server_2</code>'s origin is user-controlled.
51+
52+
</p>
53+
54+
<sample src="examples/CorsPermissiveConfigurationBad.js"/>
55+
56+
<p>
57+
58+
In the example below, the <code>server_1</code> CORS is restrictive so it's not
59+
vulnerable to CSRF attacks. And <code>server_2</code>'s is using properly sanitized
60+
user-controlled data.
61+
62+
</p>
63+
64+
<sample src="examples/CorsPermissiveConfigurationGood.js"/>
65+
</example>
66+
67+
<references>
68+
<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>
69+
<li>W3C: <a href="https://w3c.github.io/webappsec-cors-for-developers/#resources">CORS for developers, Advice for Resource Owners</a></li>
70+
</references>
71+
</qhelp>

‎javascript/ql/src/Security/CWE-942/CorsPermissiveConfiguration.ql renamed to ‎javascript/ql/src/experimental/Security/CWE-942/CorsPermissiveConfiguration.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,5 @@ import DataFlow::PathGraph
1616

1717
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
1818
where cfg.hasFlowPath(source, sink)
19-
select sink.getNode(), source, sink, "$@ misconfiguration due to a $@.", sink.getNode(),
20-
"CORS Origin", source.getNode(), "too permissive or user controlled value"
19+
select sink.getNode(), source, sink, "CORS Origin misconfiguration due to a $@.", source.getNode(),
20+
"too permissive or user controlled value"

‎javascript/ql/src/experimental/Security/CWE-942/examples/CorsPermissiveConfigurationBad.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { ApolloServer } from 'apollo-server';
2+
var https = require('https'),
3+
url = require('url');
4+
5+
var server = https.createServer(function () { });
6+
7+
server.on('request', function (req, res) {
8+
// BAD: origin is too permissive
9+
const server_1 = new ApolloServer({
10+
cors: { origin: true }
11+
});
12+
13+
let user_origin = url.parse(req.url, true).query.origin;
14+
// BAD: CORS is controlled by user
15+
const server_2 = new ApolloServer({
16+
cors: { origin: user_origin }
17+
});
18+
});

‎javascript/ql/src/experimental/Security/CWE-942/examples/CorsPermissiveConfigurationGood.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { ApolloServer } from 'apollo-server';
2+
var https = require('https'),
3+
url = require('url');
4+
5+
var server = https.createServer(function () { });
6+
7+
server.on('request', function (req, res) {
8+
// GOOD: origin is restrictive
9+
const server_1 = new ApolloServer({
10+
cors: { origin: false }
11+
});
12+
13+
let user_origin = url.parse(req.url, true).query.origin;
14+
// GOOD: user data is properly sanitized
15+
const server_2 = new ApolloServer({
16+
cors: { origin: (user_origin === "https://allowed1.com" || user_origin === "https://allowed2.com") ? user_origin : false }
17+
});
18+
});

‎javascript/ql/test/experimental/Security/CWE-942/CorsPermissiveConfiguration.expected

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
nodes
2+
| tst.js:8:9:8:59 | user_origin |
3+
| tst.js:8:23:8:46 | url.par ... , true) |
4+
| tst.js:8:23:8:52 | url.par ... ).query |
5+
| tst.js:8:23:8:59 | url.par ... .origin |
6+
| tst.js:8:33:8:39 | req.url |
7+
| tst.js:8:33:8:39 | req.url |
8+
| tst.js:8:42:8:45 | true |
9+
| tst.js:8:42:8:45 | true |
10+
| tst.js:11:25:11:28 | true |
11+
| tst.js:11:25:11:28 | true |
12+
| tst.js:11:25:11:28 | true |
13+
| tst.js:21:25:21:28 | null |
14+
| tst.js:21:25:21:28 | null |
15+
| tst.js:21:25:21:28 | null |
16+
| tst.js:26:25:26:35 | user_origin |
17+
| tst.js:26:25:26:35 | user_origin |
18+
edges
19+
| tst.js:8:9:8:59 | user_origin | tst.js:26:25:26:35 | user_origin |
20+
| tst.js:8:9:8:59 | user_origin | tst.js:26:25:26:35 | user_origin |
21+
| tst.js:8:23:8:46 | url.par ... , true) | tst.js:8:23:8:52 | url.par ... ).query |
22+
| tst.js:8:23:8:52 | url.par ... ).query | tst.js:8:23:8:59 | url.par ... .origin |
23+
| tst.js:8:23:8:59 | url.par ... .origin | tst.js:8:9:8:59 | user_origin |
24+
| tst.js:8:33:8:39 | req.url | tst.js:8:23:8:46 | url.par ... , true) |
25+
| tst.js:8:33:8:39 | req.url | tst.js:8:23:8:46 | url.par ... , true) |
26+
| tst.js:8:42:8:45 | true | tst.js:8:23:8:46 | url.par ... , true) |
27+
| tst.js:8:42:8:45 | true | tst.js:8:23:8:46 | url.par ... , true) |
28+
| tst.js:11:25:11:28 | true | tst.js:11:25:11:28 | true |
29+
| tst.js:21:25:21:28 | null | tst.js:21:25:21:28 | null |
30+
#select
31+
| tst.js:11:25:11:28 | true | tst.js:11:25:11:28 | true | tst.js:11:25:11:28 | true | CORS Origin misconfiguration due to a $@. | tst.js:11:25:11:28 | true | too permissive or user controlled value |
32+
| tst.js:21:25:21:28 | null | tst.js:21:25:21:28 | null | tst.js:21:25:21:28 | null | CORS Origin misconfiguration due to a $@. | tst.js:21:25:21:28 | null | too permissive or user controlled value |
33+
| tst.js:26:25:26:35 | user_origin | tst.js:8:33:8:39 | req.url | tst.js:26:25:26:35 | user_origin | CORS Origin misconfiguration due to a $@. | tst.js:8:33:8:39 | req.url | too permissive or user controlled value |
34+
| tst.js:26:25:26:35 | user_origin | tst.js:8:42:8:45 | true | tst.js:26:25:26:35 | user_origin | CORS Origin misconfiguration due to a $@. | tst.js:8:42:8:45 | true | too permissive or user controlled value |

‎javascript/ql/test/experimental/Security/CWE-942/CorsPermissiveConfiguration.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
./experimental/Security/CWE-942/CorsPermissiveConfiguration.ql

0 commit comments

Comments
 (0)