Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JS]: Adding express-validator support #18252

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: feature
---
* Added support for `express-validator`, a middleware for Express.js that provides a way to validate incoming requests.
1 change: 1 addition & 0 deletions javascript/ql/lib/javascript.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
114 changes: 114 additions & 0 deletions javascript/ql/lib/semmle/javascript/frameworks/ExpressValidator.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/**
* 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
Fixed Show fixed Hide fixed
validator = "body"
or
// cookie('search').notEmpty().escape()
middleware = DataFlow::moduleMember("express-validator", ["cookie"]).getACall() and
Fixed Show fixed Hide fixed
validator = "cookie"
or
// header('search').notEmpty().escape()
middleware = DataFlow::moduleMember("express-validator", ["header"]).getACall() and
Fixed Show fixed Hide fixed
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()
)
GeekMasher marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* 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` function is called and it then uses `.escape()`.
*/
private predicate isSafe(DataFlow::SourceNode node) {
// Sanitizers
exists(node.getAChainedMethodCall(["escape", "isEmail", "isIn", "isInt"]))
or
// If the `query` function is called and it then uses `.notEmpty()` or `.toString()` or `.isInt()`
exists(DataFlow::SourceNode builder |
builder =
node.getAChainedMethodCall([
"notEmpty", "exists", "isArray", "isObject", "isString", "isULID",
]) and
isSafe(builder)
)
}

/**
* Holds ExpressValidator sanitizers.
*
* These are a list of source nodes that are automatically sanitized by the
* express-validator library.
*/
Fixed Show fixed Hide fixed
class ExpressValidationSanitizer extends Shared::Sanitizer {
ExpressValidationSanitizer() {
exists(ExpressValidator::MiddlewareInstance middleware |
this = middleware.getSecureRequestInputAccess()
)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import testUtilities.ConsistencyChecking
import semmle.javascript.security.dataflow.ReflectedXssQuery as ReflectedXss
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
nodes
| src/validators.js:35:21:35:62 | `<h1>Se ... !</h1>` |
| src/validators.js:35:21:35:62 | `<h1>Se ... !</h1>` |
| src/validators.js:35:39:35:54 | req.query.search |
| src/validators.js:35:39:35:54 | req.query.search |
| src/validators.js:38:21:38:60 | `<h1>Se ... !</h1>` |
| src/validators.js:38:21:38:60 | `<h1>Se ... !</h1>` |
| src/validators.js:38:39:38:52 | req.query.name |
| src/validators.js:38:39:38:52 | req.query.name |
| src/validators.js:41:21:41:60 | `<h1>Se ... !</h1>` |
| src/validators.js:41:21:41:60 | `<h1>Se ... !</h1>` |
| src/validators.js:41:39:41:52 | req.query.name |
| src/validators.js:41:39:41:52 | req.query.name |
edges
| src/validators.js:35:39:35:54 | req.query.search | src/validators.js:35:21:35:62 | `<h1>Se ... !</h1>` |
| src/validators.js:35:39:35:54 | req.query.search | src/validators.js:35:21:35:62 | `<h1>Se ... !</h1>` |
| src/validators.js:35:39:35:54 | req.query.search | src/validators.js:35:21:35:62 | `<h1>Se ... !</h1>` |
| src/validators.js:35:39:35:54 | req.query.search | src/validators.js:35:21:35:62 | `<h1>Se ... !</h1>` |
| src/validators.js:38:39:38:52 | req.query.name | src/validators.js:38:21:38:60 | `<h1>Se ... !</h1>` |
| src/validators.js:38:39:38:52 | req.query.name | src/validators.js:38:21:38:60 | `<h1>Se ... !</h1>` |
| src/validators.js:38:39:38:52 | req.query.name | src/validators.js:38:21:38:60 | `<h1>Se ... !</h1>` |
| src/validators.js:38:39:38:52 | req.query.name | src/validators.js:38:21:38:60 | `<h1>Se ... !</h1>` |
| src/validators.js:41:39:41:52 | req.query.name | src/validators.js:41:21:41:60 | `<h1>Se ... !</h1>` |
| src/validators.js:41:39:41:52 | req.query.name | src/validators.js:41:21:41:60 | `<h1>Se ... !</h1>` |
| src/validators.js:41:39:41:52 | req.query.name | src/validators.js:41:21:41:60 | `<h1>Se ... !</h1>` |
| src/validators.js:41:39:41:52 | req.query.name | src/validators.js:41:21:41:60 | `<h1>Se ... !</h1>` |
#select
| src/validators.js:35:21:35:62 | `<h1>Se ... !</h1>` | src/validators.js:35:39:35:54 | req.query.search | src/validators.js:35:21:35:62 | `<h1>Se ... !</h1>` | Cross-site scripting vulnerability due to a $@. | src/validators.js:35:39:35:54 | req.query.search | user-provided value |
| src/validators.js:38:21:38:60 | `<h1>Se ... !</h1>` | src/validators.js:38:39:38:52 | req.query.name | src/validators.js:38:21:38:60 | `<h1>Se ... !</h1>` | Cross-site scripting vulnerability due to a $@. | src/validators.js:38:39:38:52 | req.query.name | user-provided value |
| src/validators.js:41:21:41:60 | `<h1>Se ... !</h1>` | src/validators.js:41:39:41:52 | req.query.name | src/validators.js:41:21:41:60 | `<h1>Se ... !</h1>` | Cross-site scripting vulnerability due to a $@. | src/validators.js:41:39:41:52 | req.query.name | user-provided value |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Security/CWE-079/ReflectedXss.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
| src/validators.js:7:22:7:36 | query('search') | parameter | search |
| src/validators.js:10:22:10:34 | query('type') | parameter | type |
| src/validators.js:13:22:13:35 | query('email') | parameter | email |
| src/validators.js:16:22:16:34 | query('type') | parameter | type |
| src/validators.js:19:22:19:36 | query('search') | parameter | search |
| src/validators.js:23:28:23:42 | query('search') | parameter | search |
| src/validators.js:23:54:23:66 | query('type') | parameter | type |
| src/validators.js:23:77:23:90 | query('email') | parameter | email |
| src/validators.js:30:24:30:38 | query('search') | parameter | search |
| src/validators.js:33:24:33:35 | body('name') | body | name |
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import javascript

query predicate test_middleweare(
ExpressValidator::MiddlewareInstance middleware, string param_type, string param
) {
param = middleware.getSafeParameterName() and
param_type = middleware.getValidatorType()
}
Loading