From 18917a66b84819705f31b06d5af8b05060f5a6c2 Mon Sep 17 00:00:00 2001 From: Christopher Small Date: Tue, 16 Jan 2024 09:43:02 -0800 Subject: [PATCH] Security patches (#1758) * gitignore vscode settings * Fix pentest issue 1: Stored Cross-Site Scripting * Fix pentest issue 3: Reflected Cross-Site Scripting * Fix pentest issue 5: Denial of Service * Fix pentest issue 6: Path traversal; remove deprecated "build" param * Fix pentest issue 7: Path traversal; Remove "localFile" lookup feature * Fix pentest issue 11: Unavalidated Redirects and Forwards --------- Co-authored-by: Bennie Rosas --- .gitignore | 1 + .vscode/settings.json | 3 - client-admin/public/embed.html | 1 - client-participation/api/embed.js | 4 -- client-participation/api/embedPreprod.js | 4 -- file-server/app.js | 20 ++++--- server/app.ts | 3 - server/package-lock.json | 21 +++++++ server/package.json | 3 +- server/src/server.ts | 75 +++++------------------- 10 files changed, 50 insertions(+), 85 deletions(-) delete mode 100644 .vscode/settings.json diff --git a/.gitignore b/.gitignore index 3d5708e5d..c88d975ec 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ .env prod.env +.vscode/settings.json build/ diff --git a/.vscode/settings.json b/.vscode/settings.json deleted file mode 100644 index 1d1145cce..000000000 --- a/.vscode/settings.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "jest.autoRun": "off", -} diff --git a/client-admin/public/embed.html b/client-admin/public/embed.html index f0718b038..6ee64b8e7 100644 --- a/client-admin/public/embed.html +++ b/client-admin/public/embed.html @@ -101,7 +101,6 @@ data-ui_lang="fr" data-conversation_id="2arcefpshi" - data-build="2018_2_9_10_52_2_430c032" > diff --git a/client-participation/api/embed.js b/client-participation/api/embed.js index f58e8b0e1..ee4b7e5e8 100644 --- a/client-participation/api/embed.js +++ b/client-participation/api/embed.js @@ -57,8 +57,6 @@ ucsv: d.getAttribute("data-ucsv"), ucsf: d.getAttribute("data-ucsf"), - build: d.getAttribute("data-build"), - ui_lang: d.getAttribute("data-ui_lang"), subscribe_type: d.getAttribute("data-subscribe_type"), // 0 for no prompt, 1 for email prompt (1 is default) @@ -126,8 +124,6 @@ paramStrings.push("referrer="+ encodeURIComponent(document.referrer)); } - appendIfPresent("build"); - appendIfPresent("xid"); appendIfPresent("x_name"); appendIfPresent("x_profile_image_url"); diff --git a/client-participation/api/embedPreprod.js b/client-participation/api/embedPreprod.js index 49bb70b50..d576a3a09 100644 --- a/client-participation/api/embedPreprod.js +++ b/client-participation/api/embedPreprod.js @@ -57,8 +57,6 @@ ucsv: d.getAttribute("data-ucsv"), ucsf: d.getAttribute("data-ucsf"), - build: d.getAttribute("data-build"), - ui_lang: d.getAttribute("data-ui_lang"), subscribe_type: d.getAttribute("data-subscribe_type"), // 0 for no prompt, 1 for email prompt (1 is default) @@ -127,8 +125,6 @@ paramStrings.push("referrer="+ encodeURIComponent(document.referrer)); } - appendIfPresent("build"); - appendIfPresent("xid"); appendIfPresent("x_name"); appendIfPresent("x_profile_image_url"); diff --git a/file-server/app.js b/file-server/app.js index bef73b585..2c0f2fd89 100644 --- a/file-server/app.js +++ b/file-server/app.js @@ -15,14 +15,18 @@ const serve = serveStatic('build', { function setHeaders (res, filePath) { //res.setHeader('Content-Disposition', contentDisposition(path)); // - const configFile = fs.readFileSync(filePath + ".headersJson"); - const headers = JSON.parse(configFile); - const headerNames = Object.keys(headers); - if (headerNames && headerNames.length) { - res.setHeader('Pragma', null); - headerNames.forEach((name) => { - res.setHeader(name, headers[name]); - }); + try { + const configFile = fs.readFileSync(filePath + ".headersJson"); + const headers = JSON.parse(configFile); + const headerNames = Object.keys(headers); + if (headerNames && headerNames.length) { + res.setHeader('Pragma', null); + headerNames.forEach((name) => { + res.setHeader(name, headers[name]); + }); + } + } catch (e) { + console.error(e); } } diff --git a/server/app.ts b/server/app.ts index e02ab2ce7..765f31407 100644 --- a/server/app.ts +++ b/server/app.ts @@ -93,7 +93,6 @@ helpersInitialized.then( handle_GET_iip_conversation, handle_GET_implicit_conversation_generation, handle_GET_launchPrep, - handle_GET_localFile_dev_only, handle_GET_locations, handle_GET_logMaxmindResponse, handle_GET_math_pca, @@ -1632,8 +1631,6 @@ helpersInitialized.then( ) ); - app.get(/^\/localFile\/.*/, handle_GET_localFile_dev_only); - app.get("/", handle_GET_conditionalIndexFetcher); // proxy static files diff --git a/server/package-lock.json b/server/package-lock.json index cfac9709e..1438d8797 100644 --- a/server/package-lock.json +++ b/server/package-lock.json @@ -21,6 +21,7 @@ "dotenv": "^16.0.3", "express": "~3.21.2", "fb": "~1.0.2", + "html-entities": "^2.4.0", "http-proxy": "~1.18.1", "lru-cache": "~3.0.0", "mimelib": "~0.2.19", @@ -4712,6 +4713,21 @@ "node": ">=8" } }, + "node_modules/html-entities": { + "version": "2.4.0", + "resolved": "https://registry.npmjs.org/html-entities/-/html-entities-2.4.0.tgz", + "integrity": "sha512-igBTJcNNNhvZFRtm8uA6xMY6xYleeDwn3PeBCkDz7tHttv4F2hsDI2aPgNERWzvRcNYHNT3ymRaQzllmXj4YsQ==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/mdevils" + }, + { + "type": "patreon", + "url": "https://patreon.com/mdevils" + } + ] + }, "node_modules/html-escaper": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/html-escaper/-/html-escaper-2.0.2.tgz", @@ -13006,6 +13022,11 @@ "integrity": "sha512-QFLV0taWQOZtvIRIAdBChesmogZrtuXvVWsFHZTk2SU+anspqZ2vMnoLg7IE1+Uk16N19APic1BuF8bC8c2m5g==", "dev": true }, + "html-entities": { + "version": "2.4.0", + "resolved": "https://registry.npmjs.org/html-entities/-/html-entities-2.4.0.tgz", + "integrity": "sha512-igBTJcNNNhvZFRtm8uA6xMY6xYleeDwn3PeBCkDz7tHttv4F2hsDI2aPgNERWzvRcNYHNT3ymRaQzllmXj4YsQ==" + }, "html-escaper": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/html-escaper/-/html-escaper-2.0.2.tgz", diff --git a/server/package.json b/server/package.json index 94009a921..7f8b4e879 100644 --- a/server/package.json +++ b/server/package.json @@ -37,6 +37,7 @@ "dotenv": "^16.0.3", "express": "~3.21.2", "fb": "~1.0.2", + "html-entities": "^2.4.0", "http-proxy": "~1.18.1", "lru-cache": "~3.0.0", "mimelib": "~0.2.19", @@ -55,8 +56,8 @@ "request-promise": "~4.2.6", "response-time": "~2.3.1", "simple-oauth2": "~0.2.1", - "sql": "~0.34.0", "source-map-support": "~0.5.21", + "sql": "~0.34.0", "underscore": "~1.12.1", "valid-url": "~1.0.9", "winston": "~3.8.2" diff --git a/server/src/server.ts b/server/src/server.ts index 26994dc6e..41d24d183 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -31,6 +31,7 @@ import timeout from "connect-timeout"; import zlib from "zlib"; import _ from "underscore"; import pg from "pg"; +import { encode } from "html-entities"; import { METRICS_IN_RAM, addInRamMetric, MPromise } from "./utils/metered"; import CreateUser from "./auth/create-user"; @@ -1261,8 +1262,9 @@ function initializePolisHelpers() { }); // using hex since it doesn't require escaping like base64. - let dest = hexToStr(req.p.dest); - res.redirect(dest); + const dest = hexToStr(req.p.dest); + const url = new URL(dest); + res.redirect(url.pathname + url.search + url.hash); } function handle_GET_tryCookie( @@ -4488,7 +4490,7 @@ Email verified! You can close this tab or hit the back button. function createNotificationsUnsubscribeUrl(conversation_id: any, email: any) { let params = { conversation_id: conversation_id, - email: email, + email: encode(email), }; let path = "api/v3/notifications/unsubscribe"; // Element implicitly has an 'any' type because expression of type 'string | number' can't be used to index type '{}'. @@ -4503,7 +4505,7 @@ Email verified! You can close this tab or hit the back button. function createNotificationsSubscribeUrl(conversation_id: any, email: any) { let params = { conversation_id: conversation_id, - email: email, + email: encode(email), }; let path = "api/v3/notifications/subscribe"; // Element implicitly has an 'any' type because expression of type 'string | number' can't be used to index type '{}'. @@ -13127,7 +13129,6 @@ Thanks for using Polis! x_email: any; parent_url: any; dwok: any; - build: any; show_vis: any; bg_white: any; show_share: any; @@ -13170,7 +13171,6 @@ Thanks for using Polis! let x_email = req.p.x_email; let parent_url = req.p.parent_url; let dwok = req.p.dwok; - let build = req.p.build; let o: ConversationType = {}; ifDefinedSet("parent_url", req.p, o); ifDefinedSet("auth_needed_to_vote", req.p, o); @@ -13243,9 +13243,6 @@ Thanks for using Polis! if (!_.isUndefined(dwok)) { url += "&dwok=" + dwok; } - if (!_.isUndefined(build)) { - url += "&build=" + build; - } return url; } @@ -13467,11 +13464,11 @@ Thanks for using Polis! if (preloadData && preloadData.conversation) { fbMetaTagsString += ' \n'; fbMetaTagsString += ' \n'; // fbMetaTagsString += " \n"; } @@ -13543,8 +13540,7 @@ Thanks for using Polis! end: () => any; }, preloadData: { conversation?: ConversationType }, - port: string | number | undefined, - buildNumber?: string | null | undefined + port: string | number | undefined ) { let headers = { "Content-Type": "text/html", @@ -13561,12 +13557,7 @@ Thanks for using Polis! // @ts-ignore setCookieTestCookie(req, res); - if (devMode) { - buildNumber = null; - } - - let indexPath = - (buildNumber ? "/cached/" + buildNumber : "") + "/index.html"; + let indexPath = "/index.html"; let doFetch = makeFileFetcher( hostname, @@ -13611,7 +13602,7 @@ Thanks for using Polis! }); function fetchIndexForConversation( - req: { path: string; query?: { build: any } }, + req: { path: string; }, res: any ) { logger.debug("fetchIndexForConversation", req.path); @@ -13620,11 +13611,6 @@ Thanks for using Polis! if (match && match.length) { conversation_id = match[0]; } - let buildNumber: null = null; - if (req?.query?.build) { - buildNumber = req.query.build; - logger.debug("loading_build", buildNumber); - } setTimeout(function () { // Kick off requests to twitter and FB to get the share counts. @@ -13658,14 +13644,13 @@ Thanks for using Polis! req, res, preloadData, - staticFilesParticipationPort, - buildNumber + staticFilesParticipationPort ); }) .catch(function (err: any) { logger.error("polis_err_fetching_conversation_info", err); - // Argument of type '{ path: string; query?: { build: any; } | undefined; }' is not assignable to parameter of type '{ headers?: { host: any; } | undefined; path: any; pipe: (arg0: any) => void; }'. - // Property 'pipe' is missing in type '{ path: string; query?: { build: any; } | undefined; }' but required in type '{ headers?: { host: any; } | undefined; path: any; pipe: (arg0: any) => void; }'.ts(2345) + // Argument of type '{ path: string; }' is not assignable to parameter of type '{ headers?: { host: any; } | undefined; path: any; pipe: (arg0: any) => void; }'. + // Property 'pipe' is missing in type '{ path: string; }' but required in type '{ headers?: { host: any; } | undefined; path: any; pipe: (arg0: any) => void; }'. // @ts-ignore fetch404Page(req, res); }); @@ -13824,37 +13809,6 @@ Thanks for using Polis! }; })(); - function handle_GET_localFile_dev_only( - req: { path: any }, - res: { - writeHead: ( - arg0: number, - arg1?: { "Content-Type": string } | undefined - ) => void; - end: (arg0?: undefined, arg1?: string) => void; - } - ) { - const filenameParts = String(req.path).split("/"); - filenameParts.shift(); - filenameParts.shift(); - const filename = filenameParts.join("/"); - if (!devMode) { - // pretend this route doesn't exist. - return proxy(req, res); - } - fs.readFile(filename, function (error: any, content: any) { - if (error) { - res.writeHead(500); - res.end(); - } else { - res.writeHead(200, { - "Content-Type": "text/html", - }); - res.end(content, "utf-8"); - } - }); - } - function middleware_log_request_body( req: { body: any; path: string }, res: any, @@ -13989,7 +13943,6 @@ Thanks for using Polis! handle_GET_iip_conversation, handle_GET_implicit_conversation_generation, handle_GET_launchPrep, - handle_GET_localFile_dev_only, handle_GET_locations, handle_GET_logMaxmindResponse, handle_GET_math_pca,