[JS]: Adding express-validator support

javascript/ql/lib/change-notes/2024-12-09-express-validator-support.md

@@ -0,0 +1,4 @@
+---
+category: feature
+---
+* Added support for `express-validator`, a middleware for Express.js that provides a way to validate incoming requests.

javascript/ql/lib/javascript.qll

@@ -92,6 +92,7 @@ import semmle.javascript.frameworks.DigitalOcean
 import semmle.javascript.frameworks.DomEvents
 import semmle.javascript.frameworks.Electron
 import semmle.javascript.frameworks.EventEmitter
+import semmle.javascript.frameworks.ExpressValidator
 import semmle.javascript.frameworks.Files
 import semmle.javascript.frameworks.Firebase
 import semmle.javascript.frameworks.FormParsers

javascript/ql/lib/semmle/javascript/frameworks/ExpressValidator.qll

@@ -0,0 +1,117 @@
+/**
+ * Models of npm modules that are used with Express servers.
+ */
+
+import javascript
+private import semmle.javascript.frameworks.HTTP
+private import semmle.javascript.frameworks.Express
+private import semmle.javascript.security.dataflow.Xss
+
+/**
+ * Models of the express-validator npm module.
+ */
+module ExpressValidator {
+  /**
+   * The middleware instance that is used to validate the request.
+   */
+  class MiddlewareInstance extends DataFlow::InvokeNode {
+    private string validator;
+
+    MiddlewareInstance() {
+      exists(DataFlow::SourceNode middleware |
+        (
+          // query('search').notEmpty().escape()
+          middleware = DataFlow::moduleMember("express-validator", ["query", "param"]).getACall() and
+          validator = "parameter"
+          or
+          // body('search').notEmpty().escape()
+          middleware = DataFlow::moduleMember("express-validator", "body").getACall() and
+          validator = "body"
+          or
+          // cookie('search').notEmpty().escape()
+          middleware = DataFlow::moduleMember("express-validator", "cookie").getACall() and
+          validator = "cookie"
+          or
+          // header('search').notEmpty().escape()
+          middleware = DataFlow::moduleMember("express-validator", "header").getACall() and
+          validator = "header"
+        ) and
+        isSafe(middleware)
+      |
+        this = middleware
+      )
+    }
+
+    /**
+     * Gets the name iof the parameter that is safe.
+     */
+    string getSafeParameterName() { this.getArgument(0).mayHaveStringValue(result) }
+
+    /**
+     * Gets the type of the validator (parameter, body, etc)
+     */
+    string getValidatorType() { result = validator }
+
+    /**
+     * Gets the route handler that is validated.
+     */
+    Express::RouteHandler getRouteHandler() {
+      exists(Express::RouteSetup route |
+        this.getAstNode().getParent*() = route.getARouteHandlerNode().getAstNode()
+      |
+        result = route.getLastRouteHandlerNode()
+      )
+    }
+
+    /**
+     * Gets the parameter that is validated and is secure
+     */
+    DataFlow::Node getSecureRequestInputAccess() {
+      exists(Express::RequestInputAccess node |
+        // Hook up to the parameter that is validated
+        this.getRouteHandler() = node.getRouteHandler() and
+        // Check the kind of the parameter is the same as the safely escaped parameter
+        node.getKind() = this.getValidatorType() and
+        // Check if the PropertyAccess is getSafeParameterName()
+        node.(DataFlow::PropRead).getPropertyName() = this.getSafeParameterName() and
+        node.isUserControlledObject()
+      |
+        result = node
+      )
+    }
+  }
+
+  /**
+   * If the `query/body/cookie/header` functions are called, we want to check if one of the
+   * chaining method calls is a sanitizer.
+   *
+   * If a non-sanitizing functions is called, we want to recursively check if the parent is safe
+   */
+  private predicate isSafe(DataFlow::SourceNode node) {
+    // Sanitizers
+    exists(node.getAChainedMethodCall(["escape", "isEmail", "isIn", "isInt"]))
+    or
+    // Non-sanitizing chained calls
+    exists(DataFlow::SourceNode builder |
+      builder =
+        node.getAChainedMethodCall([
+            "notEmpty", "exists", "isArray", "isObject", "isString", "isULID",
+          ]) and
+      isSafe(builder)
+    )
+  }
+
+  /**
+   * The sanitizers for `express-validator` validated requests.
+   *
+   * These are a list of source nodes that are automatically sanitized by the
+   * express-validator library.
+   */
+  class ExpressValidatorSanitizer extends DataFlow::Node {
+    ExpressValidatorSanitizer() {
+      exists(ExpressValidator::MiddlewareInstance middleware |
+        this = middleware.getSecureRequestInputAccess()
+      )
+    }
+  }
+}

javascript/ql/lib/semmle/javascript/security/dataflow/ReflectedXssCustomizations.qll

@@ -127,6 +127,9 @@ module ReflectedXss {
 
   private class IsEscapedInSwitchSanitizer extends Sanitizer, Shared::IsEscapedInSwitchSanitizer { }
 
+  private class ExpressValidatorSanitizer extends Sanitizer instanceof ExpressValidator::ExpressValidatorSanitizer
+  { }
+
   /** A third-party controllable request input, considered as a flow source for reflected XSS. */
   class ThirdPartyRequestInputAccessAsSource extends Source {
     ThirdPartyRequestInputAccessAsSource() {

javascript/ql/test/library-tests/frameworks/ExpressValidator/Validators.ql

@@ -0,0 +1,25 @@
+import javascript
+import testUtilities.InlineExpectationsTest
+
+module TestConfig implements TestSig {
+  string getARelevantTag() { result = ["middleware", "secure"] }
+
+  predicate hasActualResult(Location location, string element, string tag, string value) {
+    element = "" and
+    tag = "middleware" and
+    exists(ExpressValidator::MiddlewareInstance middleware |
+      location = middleware.getAstNode().getLocation() and
+      value = middleware.getValidatorType() + "::" + middleware.getSafeParameterName()
+    )
+    or
+    element = "" and
+    tag = "secure" and
+    exists(DataFlow::Node safe, ExpressValidator::MiddlewareInstance middleware |
+      safe = middleware.getSecureRequestInputAccess() and
+      location = safe.getAstNode().getLocation() and
+      value = safe.toString()
+    )
+  }
+}
+
+import MakeTest<TestConfig>

javascript/ql/test/library-tests/frameworks/ExpressValidator/XSS.expected

@@ -0,0 +1,30 @@
+nodes
+| src/validator.js:38:21:38:62 | `<h1>Se ... !</h1>` |
+| src/validator.js:38:21:38:62 | `<h1>Se ... !</h1>` |
+| src/validator.js:38:39:38:54 | req.query.search |
+| src/validator.js:38:39:38:54 | req.query.search |
+| src/validator.js:41:21:41:60 | `<h1>Se ... !</h1>` |
+| src/validator.js:41:21:41:60 | `<h1>Se ... !</h1>` |
+| src/validator.js:41:39:41:52 | req.query.name |
+| src/validator.js:41:39:41:52 | req.query.name |
+| src/validator.js:44:21:44:60 | `<h1>Se ... !</h1>` |
+| src/validator.js:44:21:44:60 | `<h1>Se ... !</h1>` |
+| src/validator.js:44:39:44:52 | req.query.name |
+| src/validator.js:44:39:44:52 | req.query.name |
+edges
+| src/validator.js:38:39:38:54 | req.query.search | src/validator.js:38:21:38:62 | `<h1>Se ... !</h1>` |
+| src/validator.js:38:39:38:54 | req.query.search | src/validator.js:38:21:38:62 | `<h1>Se ... !</h1>` |
+| src/validator.js:38:39:38:54 | req.query.search | src/validator.js:38:21:38:62 | `<h1>Se ... !</h1>` |
+| src/validator.js:38:39:38:54 | req.query.search | src/validator.js:38:21:38:62 | `<h1>Se ... !</h1>` |
+| src/validator.js:41:39:41:52 | req.query.name | src/validator.js:41:21:41:60 | `<h1>Se ... !</h1>` |
+| src/validator.js:41:39:41:52 | req.query.name | src/validator.js:41:21:41:60 | `<h1>Se ... !</h1>` |
+| src/validator.js:41:39:41:52 | req.query.name | src/validator.js:41:21:41:60 | `<h1>Se ... !</h1>` |
+| src/validator.js:41:39:41:52 | req.query.name | src/validator.js:41:21:41:60 | `<h1>Se ... !</h1>` |
+| src/validator.js:44:39:44:52 | req.query.name | src/validator.js:44:21:44:60 | `<h1>Se ... !</h1>` |
+| src/validator.js:44:39:44:52 | req.query.name | src/validator.js:44:21:44:60 | `<h1>Se ... !</h1>` |
+| src/validator.js:44:39:44:52 | req.query.name | src/validator.js:44:21:44:60 | `<h1>Se ... !</h1>` |
+| src/validator.js:44:39:44:52 | req.query.name | src/validator.js:44:21:44:60 | `<h1>Se ... !</h1>` |
+#select
+| src/validator.js:38:21:38:62 | `<h1>Se ... !</h1>` | src/validator.js:38:39:38:54 | req.query.search | src/validator.js:38:21:38:62 | `<h1>Se ... !</h1>` | Cross-site scripting vulnerability due to a $@. | src/validator.js:38:39:38:54 | req.query.search | user-provided value |
+| src/validator.js:41:21:41:60 | `<h1>Se ... !</h1>` | src/validator.js:41:39:41:52 | req.query.name | src/validator.js:41:21:41:60 | `<h1>Se ... !</h1>` | Cross-site scripting vulnerability due to a $@. | src/validator.js:41:39:41:52 | req.query.name | user-provided value |
+| src/validator.js:44:21:44:60 | `<h1>Se ... !</h1>` | src/validator.js:44:39:44:52 | req.query.name | src/validator.js:44:21:44:60 | `<h1>Se ... !</h1>` | Cross-site scripting vulnerability due to a $@. | src/validator.js:44:39:44:52 | req.query.name | user-provided value |

javascript/ql/test/library-tests/frameworks/ExpressValidator/XSS.qlref

@@ -0,0 +1 @@
+Security/CWE-079/ReflectedXss.ql

javascript/ql/test/library-tests/frameworks/ExpressValidator/src/validator.js

@@ -0,0 +1,50 @@
+
+const express = require('express')
+const { query, body } = require('express-validator');
+
+const app = express();
+
+app.get('/secure/1', query('search').escape(), (req, res) => {      // $ middleware=parameter::search
+    return res.send(`<h1>Searching: ${req.query.search}!</h1>`);    // $ secure=req.query.search
+})
+app.get('/secure/2', query('type').isInt(), (req, res) => {         // $ middleware=parameter::type
+    return res.send(`<h1>Type: ${req.query.type}!</h1>`);           // $ secure=req.query.type
+})
+app.get('/secure/3', query('email').isEmail(), (req, res) => {      // $ middleware=parameter::email
+    return res.send(`<h1>Email: ${req.query.email}!</h1>`);         // $ secure=req.query.email
+})
+app.get('/secure/4', query('type').isIn(["this", "or", "that"]), (req, res) => {    // $ middleware=parameter::type
+    return res.send(`<h1>Type: ${req.query.type}!</h1>`);                           // $ secure=req.query.type
+})
+app.get('/secure/5', query('search').notEmpty().isString().escape(), (req, res) => {    // $ middleware=parameter::search
+    return res.send(`<h1>Searching: ${req.query.search}!</h1>`);                        // $ secure=req.query.search
+})
+
+app.get(
+    '/multichains/1',
+    [
+        query('search').escape(),       // $ middleware=parameter::search
+        query('type').isInt(),          // $ middleware=parameter::type
+        query('email').isEmail()        // $ middleware=parameter::email
+    ],
+    (req, res) => {
+        return res.send(
+            `<h1>Searching: ${req.query.search} - ${req.query.type}</h1><h2>User: ${req.query.email}</h2>`  // $ secure=req.query.search secure=req.query.type secure=req.query.email
+        );        
+    }
+)
+
+app.get('/insecure/1', (req, res) => {
+    return res.send(`<h1>Searching: ${req.query.search}!</h1>`);
+})
+app.get('/insecure/2', query('search').escape(), (req, res) => {        // $ middleware=parameter::search
+    return res.send(`<h1>Searching: ${req.query.name}!</h1>`);
+})
+app.get('/insecure/3', body('name').escape(), (req, res) => {           // $ middleware=body::name
+    return res.send(`<h1>Searching: ${req.query.name}!</h1>`);
+})
+
+
+app.listen(8000, () => {
+    console.log(`Example app listening on port 8000`)
+})