Skip to content

Commit 9b84a8e

Browse files
codeql-cicodeql-ci
authored
Merge pull request github#6048 from erik-krogh/graphql
Approved by esbena
2 parents 0ddeb7a + 60920c1 commit 9b84a8e

File tree

7 files changed

+372
-10
lines changed

7 files changed

+372
-10
lines changed

‎javascript/change-notes/2021-06-09-graphql.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
lgtm,codescanning
2+
* The `js/sql-injection` query now recognizes graphql injections.
3+
Affected packages are
4+
[@octokit/core](https://npmjs.com/package/@octokit/core),
5+
[@octokit/rest](https://npmjs.com/package/@octokit/rest),
6+
[@octokit/graphql](https://npmjs.com/package/@octokit/graphql),
7+
[@octokit/request](https://npmjs.com/package/@octokit/request),
8+
[@actions/github](https://npmjs.com/package/@actions/github), and
9+
[graphql](https://npmjs.com/package/graphql)

‎javascript/ql/src/javascript.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ import semmle.javascript.frameworks.EventEmitter
9090
import semmle.javascript.frameworks.Files
9191
import semmle.javascript.frameworks.Firebase
9292
import semmle.javascript.frameworks.FormParsers
93+
import semmle.javascript.frameworks.GraphQL
9394
import semmle.javascript.frameworks.jQuery
9495
import semmle.javascript.frameworks.JWT
9596
import semmle.javascript.frameworks.Handlebars

‎javascript/ql/src/semmle/javascript/frameworks/GraphQL.qll

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/**
2+
* Provides classes for working with GraphQL connectors.
3+
*/
4+
5+
import javascript
6+
7+
/** Provides classes modelling concepts of GraphQL connectors. */
8+
module GraphQL {
9+
/** A string-valued expression that is interpreted as a GraphQL query. */
10+
abstract class GraphQLString extends DataFlow::Node { }
11+
}
12+
13+
/**
14+
* Provides classes modelling the octokit packages [@octokit/core](https://npmjs.com/package/@octokit/core),
15+
* [@octokit/graphql](https://npmjs.com/package/@octokit/graphql), [@octokit/rest](https://npmjs.com/package/@octokit/rest),
16+
* and [@octokit/request](https://npmjs.com/package/@octokit/request).
17+
*/
18+
private module Octokit {
19+
/** Get an instanceof of `Octokit` */
20+
private API::Node octokit() {
21+
result =
22+
API::moduleImport(["@octokit/core", "@octokit/rest"]).getMember("Octokit").getInstance()
23+
or
24+
result = API::moduleImport("@actions/github").getMember("getOctokit").getReturn()
25+
}
26+
27+
/**
28+
* Gets a reference to a `graphql` function from a `octokit` package.
29+
*/
30+
private API::Node graphQLCallee() {
31+
result = API::moduleImport(["@octokit/graphql", "@octokit/core"]).getMember("graphql")
32+
or
33+
result = octokit().getMember("graphql")
34+
or
35+
result = API::moduleImport("@octokit/graphql").getMember("withCustomRequest").getReturn()
36+
or
37+
result = graphQLCallee().getMember("defaults").getReturn()
38+
}
39+
40+
/**
41+
* Gets a reference to a `request` function from a `octokit` package.
42+
*/
43+
private API::Node requestCallee() {
44+
result =
45+
API::moduleImport(["@octokit/core", "@octokit/request", "@octokit/graphql"])
46+
.getMember("request")
47+
or
48+
result = octokit().getMember("request")
49+
or
50+
result = requestCallee().getMember("defaults").getReturn()
51+
}
52+
53+
/** A string that is interpreted as a GraphQL query by a `octokit` package. */
54+
private class GraphQLString extends GraphQL::GraphQLString {
55+
GraphQLString() { this = graphQLCallee().getACall().getArgument(0) }
56+
}
57+
58+
/**
59+
* A call to `request` seen as a client request.
60+
* E.g. `await request("POST /graphql", { query: {...data} });`
61+
*/
62+
private class RequestClientRequest extends ClientRequest::Range, API::CallNode {
63+
RequestClientRequest() { this = requestCallee().getACall() }
64+
65+
override DataFlow::Node getUrl() {
66+
result = this.getArgument(0) // contains both the method and the URL, but it's close enough
67+
}
68+
69+
override DataFlow::Node getHost() { none() }
70+
71+
override DataFlow::Node getADataNode() { result = this.getArgument(1) }
72+
}
73+
}
74+
75+
/**
76+
* Provides classes modelling [graphql](https://npmjs.com/package/graphql).
77+
*/
78+
private module GraphQLLib {
79+
/** A string that is interpreted as a GraphQL query by a `graphql` package. */
80+
private class GraphQLString extends GraphQL::GraphQLString {
81+
GraphQLString() {
82+
this = API::moduleImport("graphql").getMember("graphql").getACall().getArgument(1)
83+
}
84+
}
85+
86+
/**
87+
* A client request that appears to be a GraphQL query.
88+
* Using a client-request in this way to execute GraphQL is documented by e.g:
89+
* - [graphql](https://graphql.org/graphql-js/graphql-clients/)
90+
* - [shopify](https://shopify.dev/tutorials/graphql-with-node-and-express)
91+
* - [@octokit/request](https://npmjs.com/package/@octokit/request)
92+
*/
93+
private class GraphQLRequest extends GraphQL::GraphQLString {
94+
GraphQLRequest() {
95+
exists(ClientRequest req |
96+
this =
97+
[req.getADataNode(), req.getADataNode().(JsonStringifyCall).getInput()]
98+
.getALocalSource()
99+
.getAPropertyWrite("query")
100+
.getRhs()
101+
|
102+
containsGraphQLIndicator(req.getUrl())
103+
)
104+
}
105+
}
106+
107+
/**
108+
* Holds if `node` is a node that likely contains an URL to a GraphQL endpoint.
109+
*/
110+
private predicate containsGraphQLIndicator(DataFlow::Node node) {
111+
node.getStringValue().regexpMatch("(?i).*graphql.*")
112+
or
113+
node.(DataFlow::PropRead).getPropertyName().regexpMatch("(?i).*graphql.*")
114+
or
115+
containsGraphQLIndicator(node.(StringOps::Concatenation).getAnOperand())
116+
or
117+
containsGraphQLIndicator(node.getAPredecessor())
118+
}
119+
}

‎javascript/ql/src/semmle/javascript/security/dataflow/SqlInjection.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
2-
* Provides a taint tracking configuration for reasoning about SQL
3-
* injection vulnerabilities
2+
* Provides a taint tracking configuration for reasoning about string based
3+
* query injection vulnerabilities
44
*
55
* Note, for performance reasons: only import this file if
66
* `SqlInjection::Configuration` is needed, otherwise
@@ -13,7 +13,7 @@ module SqlInjection {
1313
import SqlInjectionCustomizations::SqlInjection
1414

1515
/**
16-
* A taint-tracking configuration for reasoning about SQL injection vulnerabilities.
16+
* A taint-tracking configuration for reasoning about string based query injection vulnerabilities.
1717
*/
1818
class Configuration extends TaintTracking::Configuration {
1919
Configuration() { this = "SqlInjection" }

‎javascript/ql/src/semmle/javascript/security/dataflow/SqlInjectionCustomizations.qll

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,28 @@
11
/**
22
* Provides default sources, sinks and sanitizers for reasoning about
3-
* SQL injection vulnerabilities, as well as extension points for
4-
* adding your own.
3+
* string based query injection vulnerabilities, as well as extension
4+
* points for adding your own.
55
*/
66

77
import javascript
88

99
module SqlInjection {
1010
/**
11-
* A data flow source for SQL injection vulnerabilities.
11+
* A data flow source for string based query injection vulnerabilities.
1212
*/
1313
abstract class Source extends DataFlow::Node { }
1414

1515
/**
16-
* A data flow sink for SQL injection vulnerabilities.
16+
* A data flow sink for string based query injection vulnerabilities.
1717
*/
1818
abstract class Sink extends DataFlow::Node { }
1919

2020
/**
21-
* A sanitizer for SQL injection vulnerabilities.
21+
* A sanitizer for string based query injection vulnerabilities.
2222
*/
2323
abstract class Sanitizer extends DataFlow::Node { }
2424

25-
/** A source of remote user input, considered as a flow source for SQL injection. */
25+
/** A source of remote user input, considered as a flow source for string based query injection. */
2626
class RemoteFlowSourceAsSource extends Source {
2727
RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
2828
}
@@ -32,8 +32,13 @@ module SqlInjection {
3232
override SQL::SqlString astNode;
3333
}
3434

35-
/** An expression that sanitizes a value for the purposes of SQL injection. */
35+
/** An expression that sanitizes a value for the purposes of string based query injection. */
3636
class SanitizerExpr extends Sanitizer, DataFlow::ValueNode {
3737
SanitizerExpr() { astNode = any(SQL::SqlSanitizer ss).getOutput() }
3838
}
39+
40+
/** An GraphQL expression passed to an API call that executes GraphQL. */
41+
class GraphqlInjectionSink extends Sink {
42+
GraphqlInjectionSink() { this instanceof GraphQL::GraphQLString }
43+
}
3944
}

‎javascript/ql/test/query-tests/Security/CWE-089/untyped/SqlInjection.expected

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,55 @@
11
nodes
2+
| graphql.js:8:11:8:28 | id |
3+
| graphql.js:8:16:8:28 | req.params.id |
4+
| graphql.js:8:16:8:28 | req.params.id |
5+
| graphql.js:10:34:20:5 | `\\n ... }\\n ` |
6+
| graphql.js:10:34:20:5 | `\\n ... }\\n ` |
7+
| graphql.js:12:46:12:47 | id |
8+
| graphql.js:26:11:26:28 | id |
9+
| graphql.js:26:16:26:28 | req.params.id |
10+
| graphql.js:26:16:26:28 | req.params.id |
11+
| graphql.js:27:30:27:40 | `foo ${id}` |
12+
| graphql.js:27:30:27:40 | `foo ${id}` |
13+
| graphql.js:27:37:27:38 | id |
14+
| graphql.js:30:32:30:42 | `foo ${id}` |
15+
| graphql.js:30:32:30:42 | `foo ${id}` |
16+
| graphql.js:30:39:30:40 | id |
17+
| graphql.js:33:18:33:28 | `foo ${id}` |
18+
| graphql.js:33:18:33:28 | `foo ${id}` |
19+
| graphql.js:33:25:33:26 | id |
20+
| graphql.js:39:11:39:28 | id |
21+
| graphql.js:39:16:39:28 | req.params.id |
22+
| graphql.js:39:16:39:28 | req.params.id |
23+
| graphql.js:44:14:44:24 | `foo ${id}` |
24+
| graphql.js:44:14:44:24 | `foo ${id}` |
25+
| graphql.js:44:21:44:22 | id |
26+
| graphql.js:48:44:48:54 | `foo ${id}` |
27+
| graphql.js:48:44:48:54 | `foo ${id}` |
28+
| graphql.js:48:51:48:52 | id |
29+
| graphql.js:55:11:55:28 | id |
30+
| graphql.js:55:16:55:28 | req.params.id |
31+
| graphql.js:55:16:55:28 | req.params.id |
32+
| graphql.js:56:39:56:49 | `foo ${id}` |
33+
| graphql.js:56:39:56:49 | `foo ${id}` |
34+
| graphql.js:56:46:56:47 | id |
35+
| graphql.js:58:66:58:76 | `foo ${id}` |
36+
| graphql.js:58:66:58:76 | `foo ${id}` |
37+
| graphql.js:58:73:58:74 | id |
38+
| graphql.js:74:9:74:25 | id |
39+
| graphql.js:74:14:74:25 | req.query.id |
40+
| graphql.js:74:14:74:25 | req.query.id |
41+
| graphql.js:75:46:75:64 | "{ foo" + id + " }" |
42+
| graphql.js:75:46:75:64 | "{ foo" + id + " }" |
43+
| graphql.js:75:56:75:57 | id |
44+
| graphql.js:84:14:90:8 | `{\\n ... }` |
45+
| graphql.js:84:14:90:8 | `{\\n ... }` |
46+
| graphql.js:88:13:88:14 | id |
47+
| graphql.js:119:11:119:28 | id |
48+
| graphql.js:119:16:119:28 | req.params.id |
49+
| graphql.js:119:16:119:28 | req.params.id |
50+
| graphql.js:120:38:120:48 | `foo ${id}` |
51+
| graphql.js:120:38:120:48 | `foo ${id}` |
52+
| graphql.js:120:45:120:46 | id |
253
| json-schema-validator.js:25:15:25:48 | query |
354
| json-schema-validator.js:25:23:25:48 | JSON.pa ... y.data) |
455
| json-schema-validator.js:25:34:25:47 | req.query.data |
@@ -332,6 +383,51 @@ nodes
332383
| tst.js:10:46:10:58 | req.params.id |
333384
| tst.js:10:46:10:58 | req.params.id |
334385
edges
386+
| graphql.js:8:11:8:28 | id | graphql.js:12:46:12:47 | id |
387+
| graphql.js:8:16:8:28 | req.params.id | graphql.js:8:11:8:28 | id |
388+
| graphql.js:8:16:8:28 | req.params.id | graphql.js:8:11:8:28 | id |
389+
| graphql.js:12:46:12:47 | id | graphql.js:10:34:20:5 | `\\n ... }\\n ` |
390+
| graphql.js:12:46:12:47 | id | graphql.js:10:34:20:5 | `\\n ... }\\n ` |
391+
| graphql.js:26:11:26:28 | id | graphql.js:27:37:27:38 | id |
392+
| graphql.js:26:11:26:28 | id | graphql.js:30:39:30:40 | id |
393+
| graphql.js:26:11:26:28 | id | graphql.js:33:25:33:26 | id |
394+
| graphql.js:26:16:26:28 | req.params.id | graphql.js:26:11:26:28 | id |
395+
| graphql.js:26:16:26:28 | req.params.id | graphql.js:26:11:26:28 | id |
396+
| graphql.js:27:37:27:38 | id | graphql.js:27:30:27:40 | `foo ${id}` |
397+
| graphql.js:27:37:27:38 | id | graphql.js:27:30:27:40 | `foo ${id}` |
398+
| graphql.js:30:39:30:40 | id | graphql.js:30:32:30:42 | `foo ${id}` |
399+
| graphql.js:30:39:30:40 | id | graphql.js:30:32:30:42 | `foo ${id}` |
400+
| graphql.js:33:25:33:26 | id | graphql.js:33:18:33:28 | `foo ${id}` |
401+
| graphql.js:33:25:33:26 | id | graphql.js:33:18:33:28 | `foo ${id}` |
402+
| graphql.js:39:11:39:28 | id | graphql.js:44:21:44:22 | id |
403+
| graphql.js:39:11:39:28 | id | graphql.js:48:51:48:52 | id |
404+
| graphql.js:39:16:39:28 | req.params.id | graphql.js:39:11:39:28 | id |
405+
| graphql.js:39:16:39:28 | req.params.id | graphql.js:39:11:39:28 | id |
406+
| graphql.js:44:21:44:22 | id | graphql.js:44:14:44:24 | `foo ${id}` |
407+
| graphql.js:44:21:44:22 | id | graphql.js:44:14:44:24 | `foo ${id}` |
408+
| graphql.js:48:51:48:52 | id | graphql.js:48:44:48:54 | `foo ${id}` |
409+
| graphql.js:48:51:48:52 | id | graphql.js:48:44:48:54 | `foo ${id}` |
410+
| graphql.js:55:11:55:28 | id | graphql.js:56:46:56:47 | id |
411+
| graphql.js:55:11:55:28 | id | graphql.js:58:73:58:74 | id |
412+
| graphql.js:55:16:55:28 | req.params.id | graphql.js:55:11:55:28 | id |
413+
| graphql.js:55:16:55:28 | req.params.id | graphql.js:55:11:55:28 | id |
414+
| graphql.js:56:46:56:47 | id | graphql.js:56:39:56:49 | `foo ${id}` |
415+
| graphql.js:56:46:56:47 | id | graphql.js:56:39:56:49 | `foo ${id}` |
416+
| graphql.js:58:73:58:74 | id | graphql.js:58:66:58:76 | `foo ${id}` |
417+
| graphql.js:58:73:58:74 | id | graphql.js:58:66:58:76 | `foo ${id}` |
418+
| graphql.js:74:9:74:25 | id | graphql.js:75:56:75:57 | id |
419+
| graphql.js:74:9:74:25 | id | graphql.js:88:13:88:14 | id |
420+
| graphql.js:74:14:74:25 | req.query.id | graphql.js:74:9:74:25 | id |
421+
| graphql.js:74:14:74:25 | req.query.id | graphql.js:74:9:74:25 | id |
422+
| graphql.js:75:56:75:57 | id | graphql.js:75:46:75:64 | "{ foo" + id + " }" |
423+
| graphql.js:75:56:75:57 | id | graphql.js:75:46:75:64 | "{ foo" + id + " }" |
424+
| graphql.js:88:13:88:14 | id | graphql.js:84:14:90:8 | `{\\n ... }` |
425+
| graphql.js:88:13:88:14 | id | graphql.js:84:14:90:8 | `{\\n ... }` |
426+
| graphql.js:119:11:119:28 | id | graphql.js:120:45:120:46 | id |
427+
| graphql.js:119:16:119:28 | req.params.id | graphql.js:119:11:119:28 | id |
428+
| graphql.js:119:16:119:28 | req.params.id | graphql.js:119:11:119:28 | id |
429+
| graphql.js:120:45:120:46 | id | graphql.js:120:38:120:48 | `foo ${id}` |
430+
| graphql.js:120:45:120:46 | id | graphql.js:120:38:120:48 | `foo ${id}` |
335431
| json-schema-validator.js:25:15:25:48 | query | json-schema-validator.js:33:22:33:26 | query |
336432
| json-schema-validator.js:25:15:25:48 | query | json-schema-validator.js:33:22:33:26 | query |
337433
| json-schema-validator.js:25:15:25:48 | query | json-schema-validator.js:35:18:35:22 | query |
@@ -740,6 +836,17 @@ edges
740836
| tst.js:10:46:10:58 | req.params.id | tst.js:10:10:10:64 | 'SELECT ... d + '"' |
741837
| tst.js:10:46:10:58 | req.params.id | tst.js:10:10:10:64 | 'SELECT ... d + '"' |
742838
#select
839+
| graphql.js:10:34:20:5 | `\\n ... }\\n ` | graphql.js:8:16:8:28 | req.params.id | graphql.js:10:34:20:5 | `\\n ... }\\n ` | This query depends on $@. | graphql.js:8:16:8:28 | req.params.id | a user-provided value |
840+
| graphql.js:27:30:27:40 | `foo ${id}` | graphql.js:26:16:26:28 | req.params.id | graphql.js:27:30:27:40 | `foo ${id}` | This query depends on $@. | graphql.js:26:16:26:28 | req.params.id | a user-provided value |
841+
| graphql.js:30:32:30:42 | `foo ${id}` | graphql.js:26:16:26:28 | req.params.id | graphql.js:30:32:30:42 | `foo ${id}` | This query depends on $@. | graphql.js:26:16:26:28 | req.params.id | a user-provided value |
842+
| graphql.js:33:18:33:28 | `foo ${id}` | graphql.js:26:16:26:28 | req.params.id | graphql.js:33:18:33:28 | `foo ${id}` | This query depends on $@. | graphql.js:26:16:26:28 | req.params.id | a user-provided value |
843+
| graphql.js:44:14:44:24 | `foo ${id}` | graphql.js:39:16:39:28 | req.params.id | graphql.js:44:14:44:24 | `foo ${id}` | This query depends on $@. | graphql.js:39:16:39:28 | req.params.id | a user-provided value |
844+
| graphql.js:48:44:48:54 | `foo ${id}` | graphql.js:39:16:39:28 | req.params.id | graphql.js:48:44:48:54 | `foo ${id}` | This query depends on $@. | graphql.js:39:16:39:28 | req.params.id | a user-provided value |
845+
| graphql.js:56:39:56:49 | `foo ${id}` | graphql.js:55:16:55:28 | req.params.id | graphql.js:56:39:56:49 | `foo ${id}` | This query depends on $@. | graphql.js:55:16:55:28 | req.params.id | a user-provided value |
846+
| graphql.js:58:66:58:76 | `foo ${id}` | graphql.js:55:16:55:28 | req.params.id | graphql.js:58:66:58:76 | `foo ${id}` | This query depends on $@. | graphql.js:55:16:55:28 | req.params.id | a user-provided value |
847+
| graphql.js:75:46:75:64 | "{ foo" + id + " }" | graphql.js:74:14:74:25 | req.query.id | graphql.js:75:46:75:64 | "{ foo" + id + " }" | This query depends on $@. | graphql.js:74:14:74:25 | req.query.id | a user-provided value |
848+
| graphql.js:84:14:90:8 | `{\\n ... }` | graphql.js:74:14:74:25 | req.query.id | graphql.js:84:14:90:8 | `{\\n ... }` | This query depends on $@. | graphql.js:74:14:74:25 | req.query.id | a user-provided value |
849+
| graphql.js:120:38:120:48 | `foo ${id}` | graphql.js:119:16:119:28 | req.params.id | graphql.js:120:38:120:48 | `foo ${id}` | This query depends on $@. | graphql.js:119:16:119:28 | req.params.id | a user-provided value |
743850
| json-schema-validator.js:33:22:33:26 | query | json-schema-validator.js:25:34:25:47 | req.query.data | json-schema-validator.js:33:22:33:26 | query | This query depends on $@. | json-schema-validator.js:25:34:25:47 | req.query.data | a user-provided value |
744851
| json-schema-validator.js:35:18:35:22 | query | json-schema-validator.js:25:34:25:47 | req.query.data | json-schema-validator.js:35:18:35:22 | query | This query depends on $@. | json-schema-validator.js:25:34:25:47 | req.query.data | a user-provided value |
745852
| json-schema-validator.js:55:22:55:26 | query | json-schema-validator.js:50:34:50:47 | req.query.data | json-schema-validator.js:55:22:55:26 | query | This query depends on $@. | json-schema-validator.js:50:34:50:47 | req.query.data | a user-provided value |

0 commit comments

Comments
 (0)