add graphql injection to the sql-injection query

2026-04-30 19:26:02 +02:00 · 2021-06-08 23:27:39 +02:00
parent e7b9603c5b
commit 50d574d20d
7 changed files with 332 additions and 10 deletions
--- a/javascript/change-notes/2021-06-09-graphql.md
+++ b/javascript/change-notes/2021-06-09-graphql.md
@@ -0,0 +1,8 @@
+lgtm,codescanning
+* The `js/sql-injection` query now recognizes graphql injections. 
+  Affected packages are
+    [@octokit/core](https://npmjs.com/package/@octokit/core),
+    [@octokit/rest](https://npmjs.com/package/@octokit/rest),
+    [@octokit/graphql](https://npmjs.com/package/@octokit/graphql),
+    [@octokit/request](https://npmjs.com/package/@octokit/request), and
+    [graphql](https://npmjs.com/package/graphql)
--- a/javascript/ql/src/javascript.qll
+++ b/javascript/ql/src/javascript.qll
@@ -90,6 +90,7 @@ import semmle.javascript.frameworks.EventEmitter
 import semmle.javascript.frameworks.Files
 import semmle.javascript.frameworks.Firebase
 import semmle.javascript.frameworks.FormParsers
+import semmle.javascript.frameworks.GraphQL
 import semmle.javascript.frameworks.jQuery
 import semmle.javascript.frameworks.JWT
 import semmle.javascript.frameworks.Handlebars
--- a/javascript/ql/src/semmle/javascript/frameworks/GraphQL.qll
+++ b/javascript/ql/src/semmle/javascript/frameworks/GraphQL.qll
@@ -0,0 +1,100 @@
+/**
+ * Provides classes for working with GraphQL connectors.
+ */
+
+import javascript
+
+/** Provides classes modelling concepts of GraphQL connectors. */
+module GraphQL {
+  /** A string-valued expression that is interpreted as a GraphQL query. */
+  abstract class GraphQLString extends DataFlow::Node { }
+}
+
+/**
+ * Provides classes modelling the octokit packages [@octokit/core](https://npmjs.com/package/@octokit/core),
+ * [@octokit/graphql](https://npmjs.com/package/@octokit/graphql), [@octokit/rest](https://npmjs.com/package/@octokit/rest),
+ * and [@octokit/request](https://npmjs.com/package/@octokit/request).
+ */
+private module Octokit {
+  /** Get an instanceof of `Octokit` */
+  private API::Node octokit() {
+    result =
+      API::moduleImport(["@octokit/core", "@octokit/rest"]).getMember("Octokit").getInstance()
+  }
+
+  /**
+   * Gets a reference to a `graphql` function from a `octokit` package.
+   */
+  private API::Node graphQLCallee() {
+    result = API::moduleImport(["@octokit/graphql", "@octokit/core"]).getMember("graphql")
+    or
+    result = octokit().getMember("graphql")
+    or
+    result = API::moduleImport("@octokit/graphql").getMember("withCustomRequest").getReturn()
+    or
+    result = graphQLCallee().getMember("defaults").getReturn()
+  }
+
+  /**
+   * Gets a reference to a `request` function from a `octokit` package.
+   */
+  private API::Node requestCallee() {
+    result =
+      API::moduleImport(["@octokit/core", "@octokit/request", "@octokit/graphql"])
+          .getMember("request")
+    or
+    result = octokit().getMember("request")
+    or
+    result = requestCallee().getMember("defaults").getReturn()
+  }
+
+  /** A string that is interpreted as a GraphQL query by a `octokit` package. */
+  private class GraphQLString extends GraphQL::GraphQLString {
+    GraphQLString() { this = graphQLCallee().getACall().getArgument(0) }
+  }
+
+  /**
+   * A call to `request` seen as a client request.
+   * E.g. `await request("POST /graphql", { query: {...data} });`
+   */
+  private class RequestClientRequest extends ClientRequest::Range, API::CallNode {
+    RequestClientRequest() { this = requestCallee().getACall() }
+
+    override DataFlow::Node getUrl() { none() }
+
+    override DataFlow::Node getHost() { none() }
+
+    override DataFlow::Node getADataNode() { result = this.getArgument(1) }
+  }
+}
+
+/**
+ * Provides classes modelling [graphql](https://npmjs.com/package/graphql).
+ */
+private module GraphQLLib {
+  /** A string that is interpreted as a GraphQL query by a `graphql` package. */
+  private class GraphQLString extends GraphQL::GraphQLString {
+    GraphQLString() {
+      this = API::moduleImport("graphql").getMember("graphql").getACall().getArgument(1)
+    }
+  }
+
+  /**
+   * A client request that appears to be a GraphQL query.
+   * Using a client-request in this way to execute GraphQL is documented by e.g:
+   *  - [graphql](https://graphql.org/graphql-js/graphql-clients/)
+   *  - [shopify](https://shopify.dev/tutorials/graphql-with-node-and-express)
+   *  - [@octokit/request](https://npmjs.com/package/@octokit/request)
+   */
+  private class GraphQLRequest extends GraphQL::GraphQLString {
+    GraphQLRequest() {
+      exists(ClientRequest req |
+        this =
+          [req.getADataNode(), req.getADataNode().(JsonStringifyCall).getInput()]
+              .getALocalSource()
+              .getAPropertyWrite("query")
+              .getRhs()
+      )
+    }
+  }
+}
--- a/javascript/ql/src/semmle/javascript/security/dataflow/SqlInjection.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/SqlInjection.qll
@@ -1,6 +1,6 @@
 /**
- * Provides a taint tracking configuration for reasoning about SQL
- * injection vulnerabilities
+ * Provides a taint tracking configuration for reasoning about string based
+ * query injection vulnerabilities
 *
 * Note, for performance reasons: only import this file if
 * `SqlInjection::Configuration` is needed, otherwise
@@ -13,7 +13,7 @@ module SqlInjection {
  import SqlInjectionCustomizations::SqlInjection

  /**
-   * A taint-tracking configuration for reasoning about SQL injection vulnerabilities.
+   * A taint-tracking configuration for reasoning about string based query injection vulnerabilities.
   */
  class Configuration extends TaintTracking::Configuration {
    Configuration() { this = "SqlInjection" }
--- a/javascript/ql/src/semmle/javascript/security/dataflow/SqlInjectionCustomizations.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/SqlInjectionCustomizations.qll
@@ -1,28 +1,28 @@
 /**
 * Provides default sources, sinks and sanitizers for reasoning about
- * SQL injection vulnerabilities, as well as extension points for
- * adding your own.
+ * string based query injection vulnerabilities, as well as extension
+ * points for adding your own.
 */

 import javascript

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

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

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

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

-  /** An expression that sanitizes a value for the purposes of SQL injection. */
+  /** An expression that sanitizes a value for the purposes of string based query injection. */
  class SanitizerExpr extends Sanitizer, DataFlow::ValueNode {
    SanitizerExpr() { astNode = any(SQL::SqlSanitizer ss).getOutput() }
  }
+
+  /** An GraphQL expression passed to an API call that executes GraphQL. */
+  class GraphqlInjectionSink extends Sink {
+    GraphqlInjectionSink() { this instanceof GraphQL::GraphQLString }
+  }
 }
--- a/javascript/ql/test/query-tests/Security/CWE-089/untyped/SqlInjection.expected
+++ b/javascript/ql/test/query-tests/Security/CWE-089/untyped/SqlInjection.expected
@@ -1,4 +1,49 @@
 nodes
+| graphql.js:8:11:8:28 | id |
+| graphql.js:8:16:8:28 | req.params.id |
+| graphql.js:8:16:8:28 | req.params.id |
+| graphql.js:10:34:20:5 | `\\n      ... }\\n    ` |
+| graphql.js:10:34:20:5 | `\\n      ... }\\n    ` |
+| graphql.js:12:46:12:47 | id |
+| graphql.js:26:11:26:28 | id |
+| graphql.js:26:16:26:28 | req.params.id |
+| graphql.js:26:16:26:28 | req.params.id |
+| graphql.js:27:30:27:40 | `foo ${id}` |
+| graphql.js:27:30:27:40 | `foo ${id}` |
+| graphql.js:27:37:27:38 | id |
+| graphql.js:30:32:30:42 | `foo ${id}` |
+| graphql.js:30:32:30:42 | `foo ${id}` |
+| graphql.js:30:39:30:40 | id |
+| graphql.js:33:18:33:28 | `foo ${id}` |
+| graphql.js:33:18:33:28 | `foo ${id}` |
+| graphql.js:33:25:33:26 | id |
+| graphql.js:39:11:39:28 | id |
+| graphql.js:39:16:39:28 | req.params.id |
+| graphql.js:39:16:39:28 | req.params.id |
+| graphql.js:44:14:44:24 | `foo ${id}` |
+| graphql.js:44:14:44:24 | `foo ${id}` |
+| graphql.js:44:21:44:22 | id |
+| graphql.js:48:44:48:54 | `foo ${id}` |
+| graphql.js:48:44:48:54 | `foo ${id}` |
+| graphql.js:48:51:48:52 | id |
+| graphql.js:55:11:55:28 | id |
+| graphql.js:55:16:55:28 | req.params.id |
+| graphql.js:55:16:55:28 | req.params.id |
+| graphql.js:56:39:56:49 | `foo ${id}` |
+| graphql.js:56:39:56:49 | `foo ${id}` |
+| graphql.js:56:46:56:47 | id |
+| graphql.js:58:66:58:76 | `foo ${id}` |
+| graphql.js:58:66:58:76 | `foo ${id}` |
+| graphql.js:58:73:58:74 | id |
+| graphql.js:74:9:74:25 | id |
+| graphql.js:74:14:74:25 | req.query.id |
+| graphql.js:74:14:74:25 | req.query.id |
+| graphql.js:75:46:75:64 | "{ foo" + id + " }" |
+| graphql.js:75:46:75:64 | "{ foo" + id + " }" |
+| graphql.js:75:56:75:57 | id |
+| graphql.js:84:14:90:8 | `{\\n     ...      }` |
+| graphql.js:84:14:90:8 | `{\\n     ...      }` |
+| graphql.js:88:13:88:14 | id |
 | json-schema-validator.js:25:15:25:48 | query |
 | json-schema-validator.js:25:23:25:48 | JSON.pa ... y.data) |
 | json-schema-validator.js:25:34:25:47 | req.query.data |
@@ -332,6 +377,46 @@ nodes
 | tst.js:10:46:10:58 | req.params.id |
 | tst.js:10:46:10:58 | req.params.id |
 edges
+| graphql.js:8:11:8:28 | id | graphql.js:12:46:12:47 | id |
+| graphql.js:8:16:8:28 | req.params.id | graphql.js:8:11:8:28 | id |
+| graphql.js:8:16:8:28 | req.params.id | graphql.js:8:11:8:28 | id |
+| graphql.js:12:46:12:47 | id | graphql.js:10:34:20:5 | `\\n      ... }\\n    ` |
+| graphql.js:12:46:12:47 | id | graphql.js:10:34:20:5 | `\\n      ... }\\n    ` |
+| graphql.js:26:11:26:28 | id | graphql.js:27:37:27:38 | id |
+| graphql.js:26:11:26:28 | id | graphql.js:30:39:30:40 | id |
+| graphql.js:26:11:26:28 | id | graphql.js:33:25:33:26 | id |
+| graphql.js:26:16:26:28 | req.params.id | graphql.js:26:11:26:28 | id |
+| graphql.js:26:16:26:28 | req.params.id | graphql.js:26:11:26:28 | id |
+| graphql.js:27:37:27:38 | id | graphql.js:27:30:27:40 | `foo ${id}` |
+| graphql.js:27:37:27:38 | id | graphql.js:27:30:27:40 | `foo ${id}` |
+| graphql.js:30:39:30:40 | id | graphql.js:30:32:30:42 | `foo ${id}` |
+| graphql.js:30:39:30:40 | id | graphql.js:30:32:30:42 | `foo ${id}` |
+| graphql.js:33:25:33:26 | id | graphql.js:33:18:33:28 | `foo ${id}` |
+| graphql.js:33:25:33:26 | id | graphql.js:33:18:33:28 | `foo ${id}` |
+| graphql.js:39:11:39:28 | id | graphql.js:44:21:44:22 | id |
+| graphql.js:39:11:39:28 | id | graphql.js:48:51:48:52 | id |
+| graphql.js:39:16:39:28 | req.params.id | graphql.js:39:11:39:28 | id |
+| graphql.js:39:16:39:28 | req.params.id | graphql.js:39:11:39:28 | id |
+| graphql.js:44:21:44:22 | id | graphql.js:44:14:44:24 | `foo ${id}` |
+| graphql.js:44:21:44:22 | id | graphql.js:44:14:44:24 | `foo ${id}` |
+| graphql.js:48:51:48:52 | id | graphql.js:48:44:48:54 | `foo ${id}` |
+| graphql.js:48:51:48:52 | id | graphql.js:48:44:48:54 | `foo ${id}` |
+| graphql.js:55:11:55:28 | id | graphql.js:56:46:56:47 | id |
+| graphql.js:55:11:55:28 | id | graphql.js:58:73:58:74 | id |
+| graphql.js:55:16:55:28 | req.params.id | graphql.js:55:11:55:28 | id |
+| graphql.js:55:16:55:28 | req.params.id | graphql.js:55:11:55:28 | id |
+| graphql.js:56:46:56:47 | id | graphql.js:56:39:56:49 | `foo ${id}` |
+| graphql.js:56:46:56:47 | id | graphql.js:56:39:56:49 | `foo ${id}` |
+| graphql.js:58:73:58:74 | id | graphql.js:58:66:58:76 | `foo ${id}` |
+| graphql.js:58:73:58:74 | id | graphql.js:58:66:58:76 | `foo ${id}` |
+| graphql.js:74:9:74:25 | id | graphql.js:75:56:75:57 | id |
+| graphql.js:74:9:74:25 | id | graphql.js:88:13:88:14 | id |
+| graphql.js:74:14:74:25 | req.query.id | graphql.js:74:9:74:25 | id |
+| graphql.js:74:14:74:25 | req.query.id | graphql.js:74:9:74:25 | id |
+| graphql.js:75:56:75:57 | id | graphql.js:75:46:75:64 | "{ foo" + id + " }" |
+| graphql.js:75:56:75:57 | id | graphql.js:75:46:75:64 | "{ foo" + id + " }" |
+| graphql.js:88:13:88:14 | id | graphql.js:84:14:90:8 | `{\\n     ...      }` |
+| graphql.js:88:13:88:14 | id | graphql.js:84:14:90:8 | `{\\n     ...      }` |
 | json-schema-validator.js:25:15:25:48 | query | json-schema-validator.js:33:22:33:26 | query |
 | json-schema-validator.js:25:15:25:48 | query | json-schema-validator.js:33:22:33:26 | query |
 | json-schema-validator.js:25:15:25:48 | query | json-schema-validator.js:35:18:35:22 | query |
@@ -740,6 +825,16 @@ edges
 | tst.js:10:46:10:58 | req.params.id | tst.js:10:10:10:64 | 'SELECT ... d + '"' |
 | tst.js:10:46:10:58 | req.params.id | tst.js:10:10:10:64 | 'SELECT ... d + '"' |
 #select
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
 | 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 |
 | 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 |
 | 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 |
--- a/javascript/ql/test/query-tests/Security/CWE-089/untyped/graphql.js
+++ b/javascript/ql/test/query-tests/Security/CWE-089/untyped/graphql.js
@@ -0,0 +1,113 @@
+var express = require('express');
+var app = express();
+
+import { Octokit } from "@octokit/core";
+const kit = new Octokit();
+
+app.get('/post/:id', function(req, res) {
+    const id = req.params.id;
+    // NOT OK
+    const response = kit.graphql(`
+      query {
+        repository(owner: "github", name: "${id}") {
+          object(expression: "master:foo") {
+            ... on Blob {
+              text
+            }
+          }
+        }
+      }
+    `);
+});
+
+import { graphql, withCustomRequest } from "@octokit/graphql";
+
+app.get('/user/:id/', function(req, res) {
+    const id = req.params.id;
+    const response = graphql(`foo ${id}`); // NOT OK
+
+    const myGraphql = withCustomRequest(request);
+    const response = myGraphql(`foo ${id}`); // NOT OK
+
+    const withDefaults = graphql.defaults({});
+    withDefaults(`foo ${id}`); // NOT OK
+});
+
+const { request } = require("@octokit/request");
+
+app.get('/article/:id/', async function(req, res) {
+    const id = req.params.id;
+    const result = await request("POST /graphql", {
+      headers: {
+        authorization: "token 0000000000000000000000000000000000000001",
+      },
+      query: `foo ${id}`, // NOT OK
+    });
+
+    const withDefaults = request.defaults({});
+    withDefaults("POST /graphql", { query: `foo ${id}` }); // NOT OK
+});
+
+import { Octokit as Core } from "@octokit/rest";
+const kit2 = new Core();
+
+app.get('/event/:id/', async function(req, res) {
+    const id = req.params.id;
+    const result = await kit2.graphql(`foo ${id}`); // NOT OK
+
+    const result2 = await kit2.request("POST /graphql", { query: `foo ${id}` }); // NOT OK
+});
+
+import { graphql as nativeGraphql, buildSchema }  from 'graphql';
+var schema = buildSchema(`
+  type Query {
+    hello: String
+  }
+`);
+var root = {
+  hello: () => {
+    return 'Hello world!';
+  },
+};
+
+app.get('/thing/:id', async function(req, res) {
+  const id = req.query.id;
+  const result = await nativeGraphql(schema, "{ foo" + id + " }", root); // NOT OK
+  
+  fetch("https://my-grpahql-server.com/graphql", {
+    method: "POST",
+    headers: {
+      "Content-Type": "application/json"
+    },
+    body: JSON.stringify({
+      // NOT OK
+      query: `{
+        thing {
+          name
+          url
+          ${id}
+        }
+      }`
+    })
+  })
+
+  fetch("https://my-grpahql-server.com/graphql", {
+    method: "POST",
+    headers: {
+      "Content-Type": "application/json"
+    },
+    body: JSON.stringify({
+      // OK
+      query: `{
+        thing {
+          name
+          url
+          $id
+        }
+      }`,
+      variables: {
+        id: id
+      }
+    })
+  })
+});