diff --git a/change-notes/1.26/analysis-javascript.md b/change-notes/1.26/analysis-javascript.md index ad2ff09d9c0..f035156652f 100644 --- a/change-notes/1.26/analysis-javascript.md +++ b/change-notes/1.26/analysis-javascript.md @@ -40,6 +40,7 @@ | Unsafe shell command constructed from library input (`js/shell-command-constructed-from-input`) | More results | This query now recognizes more commands where colon, dash, and underscore are used. | | Unsafe jQuery plugin (`js/unsafe-jquery-plugin`) | More results | This query now detects more unsafe uses of nested option properties. | | Client-side URL redirect (`js/client-side-unvalidated-url-redirection`) | More results | This query now recognizes some unsafe uses of `importScripts()` inside WebWorkers. | +| Missing CSRF middleware (`js/missing-token-validation`) | More results | This query now recognizes writes to cookie and session variables as potentially vulnerable to CSRF attacks. | ## Changes to libraries diff --git a/javascript/ql/src/Security/CWE-352/MissingCsrfMiddleware.ql b/javascript/ql/src/Security/CWE-352/MissingCsrfMiddleware.ql index ca09a09f3a8..d63786ba08d 100644 --- a/javascript/ql/src/Security/CWE-352/MissingCsrfMiddleware.ql +++ b/javascript/ql/src/Security/CWE-352/MissingCsrfMiddleware.ql @@ -18,8 +18,8 @@ string cookieProperty() { result = "session" or result = "cookies" or result = " /** Gets a data flow node that flows to the base of an access to `cookies`, `session`, or `user`. */ private DataFlow::SourceNode nodeLeadingToCookieAccess(DataFlow::TypeBackTracker t) { t.start() and - exists(DataFlow::PropRead value | - value = result.getAPropertyRead(cookieProperty()).getAPropertyRead() and + exists(DataFlow::PropRef value | + value = result.getAPropertyRead(cookieProperty()).getAPropertyReference() and // Ignore accesses to values that are part of a CSRF or captcha check not value.getPropertyName().regexpMatch("(?i).*(csrf|xsrf|captcha).*") and // Ignore calls like `req.session.save()` diff --git a/javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddleware.expected b/javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddleware.expected index 6cf61160de1..bc6087f4354 100644 --- a/javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddleware.expected +++ b/javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddleware.expected @@ -1,6 +1,7 @@ | MissingCsrfMiddlewareBad.js:7:9:7:22 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | MissingCsrfMiddlewareBad.js:10:26:12:1 | functio ... il"];\\n} | here | | MissingCsrfMiddlewareBad.js:17:13:17:26 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | MissingCsrfMiddlewareBad.js:25:30:27:6 | errorCa ... \\n }) | here | | MissingCsrfMiddlewareBad.js:33:13:33:26 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | MissingCsrfMiddlewareBad.js:41:30:43:6 | errorCa ... \\n }) | here | +| MissingCsrfMiddlewareBad.js:33:13:33:26 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | MissingCsrfMiddlewareBad.js:45:31:47:6 | errorCa ... \\n }) | here | | csurf_api_example.js:42:37:42:50 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | csurf_api_example.js:42:53:45:3 | functio ... e')\\n } | here | | csurf_example.js:18:9:18:22 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | csurf_example.js:31:40:34:1 | functio ... sed')\\n} | here | | lusca_example.js:9:9:9:22 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | lusca_example.js:26:42:29:1 | functio ... sed')\\n} | here | diff --git a/javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddlewareBad.js b/javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddlewareBad.js index fdcaf739451..d2d9b8bd4a1 100644 --- a/javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddlewareBad.js +++ b/javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddlewareBad.js @@ -41,4 +41,8 @@ app.post('/changeEmail', function (req, res) { app.post('/changeEmail', errorCatch(async function (req, res) { let newEmail = req.cookies["newEmail"]; })); + + app.post('/doLoginStuff', errorCatch(async function (req, res) { + req.session.user = loginStuff(req); + })); })