diff --git a/app.js b/app.js index 9e563a81d..6a969bccc 100644 --- a/app.js +++ b/app.js @@ -17,6 +17,7 @@ const session = require('express-session') const RedisStore = require('connect-redis')(session) const helmet = require('helmet') const nunjucks = require('nunjucks') +const { xss } = require('express-xss-sanitizer') const { APP_PORT, ENV, REDIS_URL, SESSION_SECRET } = require('./config') const addComponentRoutes = require('./routes/add-component') @@ -90,6 +91,7 @@ expressNunjucks(app, { app.use(express.urlencoded({ extended: true })) app.use(express.static(path.join(__dirname, 'public'))) app.use(express.json()) +app.use(xss()) // Routes app.use('/contribute/add-new-component', addComponentRoutes) diff --git a/helpers/previous-page.js b/helpers/previous-page.js index b8c104dcb..34a54b282 100644 --- a/helpers/previous-page.js +++ b/helpers/previous-page.js @@ -21,7 +21,7 @@ const previousPage = (url, session) => { // Check if there are subpages in the session for the current page if (subpage && Number.isInteger(Number(subpage))) { - if(subpage > 1) { + if (subpage > 1) { return `${urlRoot}/${currentPage}/${subpage - 1}` } diff --git a/middleware/component-session.js b/middleware/component-session.js index e28b7bec6..e60f1d5db 100644 --- a/middleware/component-session.js +++ b/middleware/component-session.js @@ -1,5 +1,7 @@ const crypto = require('crypto') +const sanitize = require('sanitize-filename') + const { MAX_ADD_ANOTHER: maxAddAnother, ADD_NEW_COMPONENT_ROUTE, @@ -21,7 +23,7 @@ const getHashedUrl = (url) => { } const getTemplate = (req) => { - let template = `${req.params.page || req.url.replace('/', '')}` + let template = sanitize(`${req.params.page || req.url.replace('/', '')}`) if (!Object.keys(COMPONENT_FORM_PAGES).includes(template)) { template = 'error' } @@ -29,7 +31,7 @@ const getTemplate = (req) => { } const getPageData = (req) => { - let pageData = `${req.params.page || req.url.replace('/', '')}` + let pageData = sanitize(`${req.params.page || req.url.replace('/', '')}`) if (!Object.keys(COMPONENT_FORM_PAGES).includes(pageData)) { pageData = {} } diff --git a/package-lock.json b/package-lock.json index 64dff08f2..9a8b56b09 100644 --- a/package-lock.json +++ b/package-lock.json @@ -18,6 +18,7 @@ "express-nunjucks": "^3.1.2", "express-rate-limit": "^7.5.0", "express-session": "^1.18.1", + "express-xss-sanitizer": "^2.0.0", "govuk-frontend": "^5.9.0", "helmet": "^8.1.0", "ioredis": "^5.6.0", @@ -27,6 +28,7 @@ "node-fetch": "2.7.0", "notifications-node-client": "^8.2.1", "nunjucks": "^3.2.4", + "sanitize-filename": "^1.6.3", "sanitize-html": "^2.15.0" }, "devDependencies": { @@ -13153,6 +13155,115 @@ "version": "2.0.0", "license": "MIT" }, + "node_modules/express-xss-sanitizer": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/express-xss-sanitizer/-/express-xss-sanitizer-2.0.0.tgz", + "integrity": "sha512-C9ZBsm3NuLzUaq7oAtU5Yboj2BQZEuhXShT1/dtUddVSXLPWUxgbpAWvaoDJ2fWH35NBE+/Ardjty9lQofLS8g==", + "license": "MIT", + "dependencies": { + "sanitize-html": "~2.13.0" + } + }, + "node_modules/express-xss-sanitizer/node_modules/dom-serializer": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/dom-serializer/-/dom-serializer-2.0.0.tgz", + "integrity": "sha512-wIkAryiqt/nV5EQKqQpo3SToSOV9J0DnbJqwK7Wv/Trc92zIAYZ4FlMu+JPFW1DfGFt81ZTCGgDEabffXeLyJg==", + "license": "MIT", + "dependencies": { + "domelementtype": "^2.3.0", + "domhandler": "^5.0.2", + "entities": "^4.2.0" + }, + "funding": { + "url": "https://github.com/cheeriojs/dom-serializer?sponsor=1" + } + }, + "node_modules/express-xss-sanitizer/node_modules/domhandler": { + "version": "5.0.3", + "resolved": "https://registry.npmjs.org/domhandler/-/domhandler-5.0.3.tgz", + "integrity": "sha512-cgwlv/1iFQiFnU96XXgROh8xTeetsnJiDsTc7TYCLFd9+/WNkIqPTxiM/8pSd8VIrhXGTf1Ny1q1hquVqDJB5w==", + "license": "BSD-2-Clause", + "dependencies": { + "domelementtype": "^2.3.0" + }, + "engines": { + "node": ">= 4" + }, + "funding": { + "url": "https://github.com/fb55/domhandler?sponsor=1" + } + }, + "node_modules/express-xss-sanitizer/node_modules/domutils": { + "version": "3.2.2", + "resolved": "https://registry.npmjs.org/domutils/-/domutils-3.2.2.tgz", + "integrity": "sha512-6kZKyUajlDuqlHKVX1w7gyslj9MPIXzIFiz/rGu35uC1wMi+kMhQwGhl4lt9unC9Vb9INnY9Z3/ZA3+FhASLaw==", + "license": "BSD-2-Clause", + "dependencies": { + "dom-serializer": "^2.0.0", + "domelementtype": "^2.3.0", + "domhandler": "^5.0.3" + }, + "funding": { + "url": "https://github.com/fb55/domutils?sponsor=1" + } + }, + "node_modules/express-xss-sanitizer/node_modules/entities": { + "version": "4.5.0", + "resolved": "https://registry.npmjs.org/entities/-/entities-4.5.0.tgz", + "integrity": "sha512-V0hjH4dGPh9Ao5p0MoRY6BVqtwCjhz6vI5LT8AJ55H+4g9/4vbHx1I54fS0XuclLhDHArPQCiMjDxjaL8fPxhw==", + "license": "BSD-2-Clause", + "engines": { + "node": ">=0.12" + }, + "funding": { + "url": "https://github.com/fb55/entities?sponsor=1" + } + }, + "node_modules/express-xss-sanitizer/node_modules/escape-string-regexp": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/escape-string-regexp/-/escape-string-regexp-4.0.0.tgz", + "integrity": "sha512-TtpcNJ3XAzx3Gq8sWRzJaVajRs0uVxA2YAkdb1jm2YkPz4G6egUFAyA3n5vtEIZefPk5Wa4UXbKuS5fKkJWdgA==", + "license": "MIT", + "engines": { + "node": ">=10" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/express-xss-sanitizer/node_modules/htmlparser2": { + "version": "8.0.2", + "resolved": "https://registry.npmjs.org/htmlparser2/-/htmlparser2-8.0.2.tgz", + "integrity": "sha512-GYdjWKDkbRLkZ5geuHs5NY1puJ+PXwP7+fHPRz06Eirsb9ugf6d8kkXav6ADhcODhFFPMIXyxkxSuMf3D6NCFA==", + "funding": [ + "https://github.com/fb55/htmlparser2?sponsor=1", + { + "type": "github", + "url": "https://github.com/sponsors/fb55" + } + ], + "license": "MIT", + "dependencies": { + "domelementtype": "^2.3.0", + "domhandler": "^5.0.3", + "domutils": "^3.0.1", + "entities": "^4.4.0" + } + }, + "node_modules/express-xss-sanitizer/node_modules/sanitize-html": { + "version": "2.13.1", + "resolved": "https://registry.npmjs.org/sanitize-html/-/sanitize-html-2.13.1.tgz", + "integrity": "sha512-ZXtKq89oue4RP7abL9wp/9URJcqQNABB5GGJ2acW1sdO8JTVl92f4ygD7Yc9Ze09VAZhnt2zegeU0tbNsdcLYg==", + "license": "MIT", + "dependencies": { + "deepmerge": "^4.2.2", + "escape-string-regexp": "^4.0.0", + "htmlparser2": "^8.0.0", + "is-plain-object": "^5.0.0", + "parse-srcset": "^1.0.2", + "postcss": "^8.3.11" + } + }, "node_modules/express/node_modules/cookie": { "version": "0.7.1", "license": "MIT", @@ -26085,6 +26196,15 @@ "version": "2.1.2", "license": "MIT" }, + "node_modules/sanitize-filename": { + "version": "1.6.3", + "resolved": "https://registry.npmjs.org/sanitize-filename/-/sanitize-filename-1.6.3.tgz", + "integrity": "sha512-y/52Mcy7aw3gRm7IrcGDFx/bCk4AhRh2eI9luHOQM86nZsqwiRkkq2GekHXBBD+SmPidc8i2PqtYZl+pWJ8Oeg==", + "license": "WTFPL OR ISC", + "dependencies": { + "truncate-utf8-bytes": "^1.0.0" + } + }, "node_modules/sanitize-html": { "version": "2.15.0", "license": "MIT", @@ -29500,6 +29620,15 @@ "node": ">=0.8.0" } }, + "node_modules/truncate-utf8-bytes": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/truncate-utf8-bytes/-/truncate-utf8-bytes-1.0.2.tgz", + "integrity": "sha512-95Pu1QXQvruGEhv62XCMO3Mm90GscOCClvrIUwCM0PYOXK3kaF3l3sIHxx71ThJfcbM2O5Au6SO3AWCSEfW4mQ==", + "license": "WTFPL", + "dependencies": { + "utf8-byte-length": "^1.0.1" + } + }, "node_modules/ts-api-utils": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/ts-api-utils/-/ts-api-utils-2.1.0.tgz", @@ -30047,6 +30176,12 @@ "dev": true, "license": "MIT" }, + "node_modules/utf8-byte-length": { + "version": "1.0.5", + "resolved": "https://registry.npmjs.org/utf8-byte-length/-/utf8-byte-length-1.0.5.tgz", + "integrity": "sha512-Xn0w3MtiQ6zoz2vFyUVruaCL53O/DwUvkEeOvj+uulMm0BkUGYWmBYVyElqZaSLhY6ZD0ulfU3aBra2aVT4xfA==", + "license": "(WTFPL OR MIT)" + }, "node_modules/util-deprecate": { "version": "1.0.2", "license": "MIT" diff --git a/package.json b/package.json index 7afdd9c06..ee3ce5bff 100644 --- a/package.json +++ b/package.json @@ -71,6 +71,7 @@ "express-nunjucks": "^3.1.2", "express-rate-limit": "^7.5.0", "express-session": "^1.18.1", + "express-xss-sanitizer": "^2.0.0", "govuk-frontend": "^5.9.0", "helmet": "^8.1.0", "ioredis": "^5.6.0", @@ -80,6 +81,7 @@ "node-fetch": "2.7.0", "notifications-node-client": "^8.2.1", "nunjucks": "^3.2.4", + "sanitize-filename": "^1.6.3", "sanitize-html": "^2.15.0" }, "devDependencies": { diff --git a/routes/add-component.js b/routes/add-component.js index a34620c5f..eeb1ad3f0 100644 --- a/routes/add-component.js +++ b/routes/add-component.js @@ -1,6 +1,7 @@ const crypto = require('crypto') const express = require('express') +const { xss } = require('express-xss-sanitizer') const multer = require('multer') const { COMPONENT_FORM_PAGES, ADD_NEW_COMPONENT_ROUTE } = require('../config') @@ -77,9 +78,6 @@ const setCsrfToken = (req, res, next) => { router.all('*', setCsrfToken) -// TODO: Should we not have a post * route that verfies the CSRF for *all* posts -// rather than relying on the chack being added to every post route declared? - // TODO: Why is this a get * ? Can it not just be set in the get /checkYourAnswersPath route below? router.get('*', (req, res, next) => { if (req?.session) { @@ -182,6 +180,7 @@ router.get( router.post( ['/remove/:page', '/remove/:page/:subpage'], + xss(), verifyCsrf, validatePageParams, removeFromSession, @@ -263,12 +262,13 @@ router.post( ['/component-image', '/component-image/:subpage'], validatePageParams, upload.single('componentImage'), + xss(), + verifyCsrf, saveFileToRedis, canAddAnother, validateFormDataFileUpload, setNextPage, getBackLink, - verifyCsrf, validateFormData, (req, res, next) => { if (req.file) { @@ -298,22 +298,15 @@ router.post( } ) -// Accessibility file upload -router.post( - ['/add-internal-audit', '/add-external-audit', '/add-assistive-tech'], - upload.single('accessibilityReport'), - saveFileToRedis, - validateFormDataFileUpload -) - // Form submissions for pages router.post( ['/:page', '/:page/:subpage'], + xss(), + verifyCsrf, validatePageParams, setNextPage, canSkipQuestion, getBackLink, - verifyCsrf, validateFormData, saveSession, (req, res, next) => { diff --git a/views/community/pages/add-assistive-tech.njk b/views/community/pages/add-assistive-tech.njk index d8382e0a0..41d009e03 100644 --- a/views/community/pages/add-assistive-tech.njk +++ b/views/community/pages/add-assistive-tech.njk @@ -23,7 +23,7 @@ errorList: errorList }) }} {% endif %} -