Merge pull request #6035 from erik-krogh/joi

Approved by asgerf
2026-04-29 10:45:15 +02:00 · 2021-06-09 04:42:54 -07:00
parent 6bdd7df810 3e171adaab
commit d65e6bbfa1
9 changed files with 144 additions and 3 deletions
--- a/javascript/ql/src/semmle/javascript/JsonSchema.qll
+++ b/javascript/ql/src/semmle/javascript/JsonSchema.qll
@@ -13,8 +13,20 @@ module JsonSchema {
    /** Gets the data flow node whose value is being validated. */
    abstract DataFlow::Node getInput();

-    /** Gets the return value that indicates successful validation. */
+    /**
+     * Gets the return value that indicates successful validation, if any.
+     *
+     * Has no result if the return value from this call does not directly
+     * indicate success.
+     */
    boolean getPolarity() { result = true }
+
+    /**
+     * Gets a value that indicates whether the validation was successful.
+     */
+    DataFlow::Node getAValidationResultAccess(boolean polarity) {
+      result = this and polarity = getPolarity()
+    }
  }

  /** A data flow node that is used a JSON schema. */
@@ -126,4 +138,55 @@ module JsonSchema {
      }
    }
  }
+
+  /** Provides a model for working with the [`joi`](https://npmjs.org/package/joi) library. */
+  module Joi {
+    /** A schema created using `joi.object()` or other schemas that might refer an object schema. */
+    private API::Node objectSchema() {
+      // A call that creates a schema that might be an object schema.
+      result =
+        API::moduleImport("joi")
+            .getMember([
+                "object", "alternatives", "all", "link", "compile", "allow", "valid", "when",
+                "build", "options"
+              ])
+            .getReturn()
+      or
+      // A call to a schema that returns another schema.
+      // Read from the [index.d.ts](https://github.com/sideway/joi/blob/master/lib/index.d.ts) file.
+      result =
+        objectSchema()
+            .getMember([
+                // AnySchema
+                "allow", "alter", "bind", "cache", "cast", "concat", "default", "description",
+                "disallow", "empty", "equal", "error", "example", "exist", "external", "failover",
+                "forbidden", "fork", "id", "invalid", "keep", "label", "message", "messages",
+                "meta", "not", "note", "only", "optional", "options", "prefs", "preferences",
+                "presence", "raw", "required", "rule", "shared", "strict", "strip", "tag", "tailor",
+                "unit", "valid", "warn", "warning", "when",
+                // ObjectSchema
+                "and", "append", "assert", "instance", "keys", "length", "max", "min", "nand", "or",
+                "oxor", "pattern", "ref", "regex", "rename", "schema", "unknown", "with", "without",
+                "xor"
+              ])
+            .getReturn()
+    }
+
+    /**
+     * A call to the `validate` method from the [`joi`](https://npmjs.org/package/joi) library.
+     * The `error` property in the result indicates whether the validation was successful.
+     */
+    class JoiValidationErrorRead extends ValidationCall, API::CallNode {
+      JoiValidationErrorRead() { this = objectSchema().getMember("validate").getACall() }
+
+      override DataFlow::Node getInput() { result = this.getArgument(0) }
+
+      override boolean getPolarity() { none() }
+
+      override DataFlow::Node getAValidationResultAccess(boolean polarity) {
+        result = this.getReturn().getMember("error").getAnImmediateUse() and
+        polarity = false
+      }
+    }
+  }
 }
--- a/javascript/ql/src/semmle/javascript/security/TaintedObject.qll
+++ b/javascript/ql/src/semmle/javascript/security/TaintedObject.qll
@@ -115,11 +115,12 @@ module TaintedObject {
   */
  private class JsonSchemaValidationGuard extends SanitizerGuard {
    JsonSchema::ValidationCall call;
+    boolean polarity;

-    JsonSchemaValidationGuard() { this = call }
+    JsonSchemaValidationGuard() { this = call.getAValidationResultAccess(polarity) }

    override predicate sanitizes(boolean outcome, Expr e, FlowLabel label) {
-      outcome = call.getPolarity() and
+      outcome = polarity and
      e = call.getInput().asExpr() and
      label = label()
    }
--- a/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll
@@ -617,6 +617,8 @@ module ExceptionXss {
  private class JsonSchemaValidationError extends Source {
    JsonSchemaValidationError() {
      this = any(JsonSchema::Ajv::Instance i).getAValidationError().getAnImmediateUse()
+      or
+      this = any(JsonSchema::Joi::JoiValidationErrorRead r).getAValidationResultAccess(_)
    }

    override string getDescription() { result = "JSON schema validation error" }
--- a/javascript/ql/test/query-tests/Security/CWE-079/ExceptionXss/ExceptionXss.expected
+++ b/javascript/ql/test/query-tests/Security/CWE-079/ExceptionXss/ExceptionXss.expected
@@ -2,6 +2,9 @@ nodes
 | ajv.js:11:18:11:33 | ajv.errorsText() |
 | ajv.js:11:18:11:33 | ajv.errorsText() |
 | ajv.js:11:18:11:33 | ajv.errorsText() |
+| ajv.js:24:18:24:26 | val.error |
+| ajv.js:24:18:24:26 | val.error |
+| ajv.js:24:18:24:26 | val.error |
 | exception-xss.js:2:6:2:28 | foo |
 | exception-xss.js:2:12:2:28 | document.location |
 | exception-xss.js:2:12:2:28 | document.location |
@@ -89,6 +92,7 @@ nodes
 | exception-xss.js:182:19:182:23 | error |
 edges
 | ajv.js:11:18:11:33 | ajv.errorsText() | ajv.js:11:18:11:33 | ajv.errorsText() |
+| ajv.js:24:18:24:26 | val.error | ajv.js:24:18:24:26 | val.error |
 | exception-xss.js:2:6:2:28 | foo | exception-xss.js:9:11:9:13 | foo |
 | exception-xss.js:2:6:2:28 | foo | exception-xss.js:15:9:15:11 | foo |
 | exception-xss.js:2:6:2:28 | foo | exception-xss.js:21:11:21:13 | foo |
@@ -170,6 +174,7 @@ edges
 | exception-xss.js:180:26:180:30 | error | exception-xss.js:182:19:182:23 | error |
 #select
 | ajv.js:11:18:11:33 | ajv.errorsText() | ajv.js:11:18:11:33 | ajv.errorsText() | ajv.js:11:18:11:33 | ajv.errorsText() | $@ is reinterpreted as HTML without escaping meta-characters. | ajv.js:11:18:11:33 | ajv.errorsText() | JSON schema validation error |
+| ajv.js:24:18:24:26 | val.error | ajv.js:24:18:24:26 | val.error | ajv.js:24:18:24:26 | val.error | $@ is reinterpreted as HTML without escaping meta-characters. | ajv.js:24:18:24:26 | val.error | JSON schema validation error |
 | exception-xss.js:11:18:11:18 | e | exception-xss.js:2:12:2:28 | document.location | exception-xss.js:11:18:11:18 | e | $@ is reinterpreted as HTML without escaping meta-characters. | exception-xss.js:2:12:2:28 | document.location | Exception text |
 | exception-xss.js:17:18:17:18 | e | exception-xss.js:2:12:2:28 | document.location | exception-xss.js:17:18:17:18 | e | $@ is reinterpreted as HTML without escaping meta-characters. | exception-xss.js:2:12:2:28 | document.location | Exception text |
 | exception-xss.js:23:18:23:18 | e | exception-xss.js:2:12:2:28 | document.location | exception-xss.js:23:18:23:18 | e | $@ is reinterpreted as HTML without escaping meta-characters. | exception-xss.js:2:12:2:28 | document.location | Exception text |
--- a/javascript/ql/test/query-tests/Security/CWE-079/ExceptionXss/ajv.js
+++ b/javascript/ql/test/query-tests/Security/CWE-079/ExceptionXss/ajv.js
@@ -11,3 +11,16 @@ app.post('/polldata', (req, res) => {
        res.send(ajv.errorsText()); // NOT OK
    }
 });
+
+const joi = require("joi");
+const joiSchema = joi.object().keys({
+    name: joi.string().required(),
+    age: joi.number().required()
+}).with('name', 'age');
+
+app.post('/votedata', (req, res) => {
+    const val = joiSchema.validate(req.body);
+    if (val.error) {
+        res.send(val.error); // NOT OK
+    }
+});
--- a/javascript/ql/test/query-tests/Security/CWE-089/untyped/DatabaseAccesses.expected
+++ b/javascript/ql/test/query-tests/Security/CWE-089/untyped/DatabaseAccesses.expected
@@ -2,6 +2,10 @@
 | json-schema-validator.js:30:13:30:27 | doc.find(query) |
 | json-schema-validator.js:33:13:33:27 | doc.find(query) |
 | json-schema-validator.js:35:9:35:23 | doc.find(query) |
+| json-schema-validator.js:53:13:53:27 | doc.find(query) |
+| json-schema-validator.js:55:13:55:27 | doc.find(query) |
+| json-schema-validator.js:59:13:59:27 | doc.find(query) |
+| json-schema-validator.js:61:13:61:27 | doc.find(query) |
 | marsdb-flow-to.js:14:3:14:22 | db.myDoc.find(query) |
 | marsdb.js:16:3:16:17 | doc.find(query) |
 | minimongo.js:18:3:18:17 | doc.find(query) |
--- a/javascript/ql/test/query-tests/Security/CWE-089/untyped/SqlInjection.expected
+++ b/javascript/ql/test/query-tests/Security/CWE-089/untyped/SqlInjection.expected
@@ -7,6 +7,16 @@ nodes
 | json-schema-validator.js:33:22:33:26 | query |
 | json-schema-validator.js:35:18:35:22 | query |
 | json-schema-validator.js:35:18:35:22 | query |
+| json-schema-validator.js:50:15:50:48 | query |
+| json-schema-validator.js:50:23:50:48 | JSON.pa ... y.data) |
+| json-schema-validator.js:50:34:50:47 | req.query.data |
+| json-schema-validator.js:50:34:50:47 | req.query.data |
+| json-schema-validator.js:55:22:55:26 | query |
+| json-schema-validator.js:55:22:55:26 | query |
+| json-schema-validator.js:59:22:59:26 | query |
+| json-schema-validator.js:59:22:59:26 | query |
+| json-schema-validator.js:61:22:61:26 | query |
+| json-schema-validator.js:61:22:61:26 | query |
 | marsdb-flow-to.js:10:9:10:18 | query |
 | marsdb-flow-to.js:10:17:10:18 | {} |
 | marsdb-flow-to.js:11:17:11:24 | req.body |
@@ -329,6 +339,15 @@ edges
 | json-schema-validator.js:25:23:25:48 | JSON.pa ... y.data) | json-schema-validator.js:25:15:25:48 | query |
 | json-schema-validator.js:25:34:25:47 | req.query.data | json-schema-validator.js:25:23:25:48 | JSON.pa ... y.data) |
 | json-schema-validator.js:25:34:25:47 | req.query.data | json-schema-validator.js:25:23:25:48 | JSON.pa ... y.data) |
+| json-schema-validator.js:50:15:50:48 | query | json-schema-validator.js:55:22:55:26 | query |
+| json-schema-validator.js:50:15:50:48 | query | json-schema-validator.js:55:22:55:26 | query |
+| json-schema-validator.js:50:15:50:48 | query | json-schema-validator.js:59:22:59:26 | query |
+| json-schema-validator.js:50:15:50:48 | query | json-schema-validator.js:59:22:59:26 | query |
+| json-schema-validator.js:50:15:50:48 | query | json-schema-validator.js:61:22:61:26 | query |
+| json-schema-validator.js:50:15:50:48 | query | json-schema-validator.js:61:22:61:26 | query |
+| json-schema-validator.js:50:23:50:48 | JSON.pa ... y.data) | json-schema-validator.js:50:15:50:48 | query |
+| json-schema-validator.js:50:34:50:47 | req.query.data | json-schema-validator.js:50:23:50:48 | JSON.pa ... y.data) |
+| json-schema-validator.js:50:34:50:47 | req.query.data | json-schema-validator.js:50:23:50:48 | JSON.pa ... y.data) |
 | marsdb-flow-to.js:10:9:10:18 | query | marsdb-flow-to.js:14:17:14:21 | query |
 | marsdb-flow-to.js:10:9:10:18 | query | marsdb-flow-to.js:14:17:14:21 | query |
 | marsdb-flow-to.js:10:17:10:18 | {} | marsdb-flow-to.js:10:9:10:18 | query |
@@ -723,6 +742,9 @@ edges
 #select
 | 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 |
+| json-schema-validator.js:59:22:59:26 | query | json-schema-validator.js:50:34:50:47 | req.query.data | json-schema-validator.js:59:22:59:26 | query | This query depends on $@. | json-schema-validator.js:50:34:50:47 | req.query.data | a user-provided value |
+| json-schema-validator.js:61:22:61:26 | query | json-schema-validator.js:50:34:50:47 | req.query.data | json-schema-validator.js:61:22:61:26 | query | This query depends on $@. | json-schema-validator.js:50:34:50:47 | req.query.data | a user-provided value |
 | marsdb-flow-to.js:14:17:14:21 | query | marsdb-flow-to.js:11:17:11:24 | req.body | marsdb-flow-to.js:14:17:14:21 | query | This query depends on $@. | marsdb-flow-to.js:11:17:11:24 | req.body | a user-provided value |
 | marsdb.js:16:12:16:16 | query | marsdb.js:13:17:13:24 | req.body | marsdb.js:16:12:16:16 | query | This query depends on $@. | marsdb.js:13:17:13:24 | req.body | a user-provided value |
 | minimongo.js:18:12:18:16 | query | minimongo.js:15:17:15:24 | req.body | minimongo.js:18:12:18:16 | query | This query depends on $@. | minimongo.js:15:17:15:24 | req.body | a user-provided value |
--- a/javascript/ql/test/query-tests/Security/CWE-089/untyped/json-schema-validator.js
+++ b/javascript/ql/test/query-tests/Security/CWE-089/untyped/json-schema-validator.js
@@ -35,3 +35,30 @@ app.post('/documents/find', (req, res) => {
        doc.find(query); // NOT OK
    });
 });
+
+import Joi from 'joi';
+
+const joiSchema = Joi.object({
+    date: Joi.string().required(),
+    title: Joi.string().required()
+}).with('date', 'title');
+
+app.post('/documents/insert', (req, res) => {
+    MongoClient.connect('mongodb://localhost:27017/test', async (err, db) => {
+        let doc = db.collection('doc');
+
+        const query = JSON.parse(req.query.data);
+        const validate = joiSchema.validate(query);
+        if (!validate.error) {
+            doc.find(query); // OK
+        } else {
+            doc.find(query); // NOT OK
+        }
+        try {
+            await joiSchema.validateAsync(query);
+            doc.find(query); // OK - but still flagged [INCONSISTENCY]
+        } catch (e) {
+            doc.find(query); // NOT OK
+        }
+    });
+});