Skip to content

Commit 50d574d

Browse files
erik-krogherik-krogh
committed
add graphql injection to the sql-injection query
1 parent e7b9603 commit 50d574d

File tree

7 files changed

+332
-10
lines changed

7 files changed

+332
-10
lines changed

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
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), and
8+
[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: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
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+
}
24+
25+
/**
26+
* Gets a reference to a `graphql` function from a `octokit` package.
27+
*/
28+
private API::Node graphQLCallee() {
29+
result = API::moduleImport(["@octokit/graphql", "@octokit/core"]).getMember("graphql")
30+
or
31+
result = octokit().getMember("graphql")
32+
or
33+
result = API::moduleImport("@octokit/graphql").getMember("withCustomRequest").getReturn()
34+
or
35+
result = graphQLCallee().getMember("defaults").getReturn()
36+
}
37+
38+
/**
39+
* Gets a reference to a `request` function from a `octokit` package.
40+
*/
41+
private API::Node requestCallee() {
42+
result =
43+
API::moduleImport(["@octokit/core", "@octokit/request", "@octokit/graphql"])
44+
.getMember("request")
45+
or
46+
result = octokit().getMember("request")
47+
or
48+
result = requestCallee().getMember("defaults").getReturn()
49+
}
50+
51+
/** A string that is interpreted as a GraphQL query by a `octokit` package. */
52+
private class GraphQLString extends GraphQL::GraphQLString {
53+
GraphQLString() { this = graphQLCallee().getACall().getArgument(0) }
54+
}
55+
56+
/**
57+
* A call to `request` seen as a client request.
58+
* E.g. `await request("POST /graphql", { query: {...data} });`
59+
*/
60+
private class RequestClientRequest extends ClientRequest::Range, API::CallNode {
61+
RequestClientRequest() { this = requestCallee().getACall() }
62+
63+
override DataFlow::Node getUrl() { none() }
64+
65+
override DataFlow::Node getHost() { none() }
66+
67+
override DataFlow::Node getADataNode() { result = this.getArgument(1) }
68+
}
69+
}
70+
71+
/**
72+
* Provides classes modelling [graphql](https://npmjs.com/package/graphql).
73+
*/
74+
private module GraphQLLib {
75+
/** A string that is interpreted as a GraphQL query by a `graphql` package. */
76+
private class GraphQLString extends GraphQL::GraphQLString {
77+
GraphQLString() {
78+
this = API::moduleImport("graphql").getMember("graphql").getACall().getArgument(1)
79+
}
80+
}
81+
82+
/**
83+
* A client request that appears to be a GraphQL query.
84+
* Using a client-request in this way to execute GraphQL is documented by e.g:
85+
* - [graphql](https://graphql.org/graphql-js/graphql-clients/)
86+
* - [shopify](https://shopify.dev/tutorials/graphql-with-node-and-express)
87+
* - [@octokit/request](https://npmjs.com/package/@octokit/request)
88+
*/
89+
private class GraphQLRequest extends GraphQL::GraphQLString {
90+
GraphQLRequest() {
91+
exists(ClientRequest req |
92+
this =
93+
[req.getADataNode(), req.getADataNode().(JsonStringifyCall).getInput()]
94+
.getALocalSource()
95+
.getAPropertyWrite("query")
96+
.getRhs()
97+
)
98+
}
99+
}
100+
}

‎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: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,49 @@
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 |
247
| json-schema-validator.js:25:15:25:48 | query |
348
| json-schema-validator.js:25:23:25:48 | JSON.pa ... y.data) |
449
| json-schema-validator.js:25:34:25:47 | req.query.data |
@@ -332,6 +377,46 @@ nodes
332377
| tst.js:10:46:10:58 | req.params.id |
333378
| tst.js:10:46:10:58 | req.params.id |
334379
edges
380+
| graphql.js:8:11:8:28 | id | graphql.js:12:46:12:47 | id |
381+
| graphql.js:8:16:8:28 | req.params.id | graphql.js:8:11:8:28 | id |
382+
| graphql.js:8:16:8:28 | req.params.id | graphql.js:8:11:8:28 | id |
383+
| graphql.js:12:46:12:47 | id | graphql.js:10:34:20:5 | `\\n ... }\\n ` |
384+
| graphql.js:12:46:12:47 | id | graphql.js:10:34:20:5 | `\\n ... }\\n ` |
385+
| graphql.js:26:11:26:28 | id | graphql.js:27:37:27:38 | id |
386+
| graphql.js:26:11:26:28 | id | graphql.js:30:39:30:40 | id |
387+
| graphql.js:26:11:26:28 | id | graphql.js:33:25:33:26 | id |
388+
| graphql.js:26:16:26:28 | req.params.id | graphql.js:26:11:26:28 | id |
389+
| graphql.js:26:16:26:28 | req.params.id | graphql.js:26:11:26:28 | id |
390+
| graphql.js:27:37:27:38 | id | graphql.js:27:30:27:40 | `foo ${id}` |
391+
| graphql.js:27:37:27:38 | id | graphql.js:27:30:27:40 | `foo ${id}` |
392+
| graphql.js:30:39:30:40 | id | graphql.js:30:32:30:42 | `foo ${id}` |
393+
| graphql.js:30:39:30:40 | id | graphql.js:30:32:30:42 | `foo ${id}` |
394+
| graphql.js:33:25:33:26 | id | graphql.js:33:18:33:28 | `foo ${id}` |
395+
| graphql.js:33:25:33:26 | id | graphql.js:33:18:33:28 | `foo ${id}` |
396+
| graphql.js:39:11:39:28 | id | graphql.js:44:21:44:22 | id |
397+
| graphql.js:39:11:39:28 | id | graphql.js:48:51:48:52 | id |
398+
| graphql.js:39:16:39:28 | req.params.id | graphql.js:39:11:39:28 | id |
399+
| graphql.js:39:16:39:28 | req.params.id | graphql.js:39:11:39:28 | id |
400+
| graphql.js:44:21:44:22 | id | graphql.js:44:14:44:24 | `foo ${id}` |
401+
| graphql.js:44:21:44:22 | id | graphql.js:44:14:44:24 | `foo ${id}` |
402+
| graphql.js:48:51:48:52 | id | graphql.js:48:44:48:54 | `foo ${id}` |
403+
| graphql.js:48:51:48:52 | id | graphql.js:48:44:48:54 | `foo ${id}` |
404+
| graphql.js:55:11:55:28 | id | graphql.js:56:46:56:47 | id |
405+
| graphql.js:55:11:55:28 | id | graphql.js:58:73:58:74 | id |
406+
| graphql.js:55:16:55:28 | req.params.id | graphql.js:55:11:55:28 | id |
407+
| graphql.js:55:16:55:28 | req.params.id | graphql.js:55:11:55:28 | id |
408+
| graphql.js:56:46:56:47 | id | graphql.js:56:39:56:49 | `foo ${id}` |
409+
| graphql.js:56:46:56:47 | id | graphql.js:56:39:56:49 | `foo ${id}` |
410+
| graphql.js:58:73:58:74 | id | graphql.js:58:66:58:76 | `foo ${id}` |
411+
| graphql.js:58:73:58:74 | id | graphql.js:58:66:58:76 | `foo ${id}` |
412+
| graphql.js:74:9:74:25 | id | graphql.js:75:56:75:57 | id |
413+
| graphql.js:74:9:74:25 | id | graphql.js:88:13:88:14 | id |
414+
| graphql.js:74:14:74:25 | req.query.id | graphql.js:74:9:74:25 | id |
415+
| graphql.js:74:14:74:25 | req.query.id | graphql.js:74:9:74:25 | id |
416+
| graphql.js:75:56:75:57 | id | graphql.js:75:46:75:64 | "{ foo" + id + " }" |
417+
| graphql.js:75:56:75:57 | id | graphql.js:75:46:75:64 | "{ foo" + id + " }" |
418+
| graphql.js:88:13:88:14 | id | graphql.js:84:14:90:8 | `{\\n ... }` |
419+
| graphql.js:88:13:88:14 | id | graphql.js:84:14:90:8 | `{\\n ... }` |
335420
| json-schema-validator.js:25:15:25:48 | query | json-schema-validator.js:33:22:33:26 | query |
336421
| json-schema-validator.js:25:15:25:48 | query | json-schema-validator.js:33:22:33:26 | query |
337422
| json-schema-validator.js:25:15:25:48 | query | json-schema-validator.js:35:18:35:22 | query |
@@ -740,6 +825,16 @@ edges
740825
| tst.js:10:46:10:58 | req.params.id | tst.js:10:10:10:64 | 'SELECT ... d + '"' |
741826
| tst.js:10:46:10:58 | req.params.id | tst.js:10:10:10:64 | 'SELECT ... d + '"' |
742827
#select
828+
| 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 |
829+
| 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 |
830+
| 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 |
831+
| 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 |
832+
| 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 |
833+
| 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 |
834+
| 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 |
835+
| 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 |
836+
| 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 |
837+
| 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 |
743838
| 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 |
744839
| 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 |
745840
| 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)