diff --git a/services/web/app/src/infrastructure/ExpressLocals.js b/services/web/app/src/infrastructure/ExpressLocals.js index 4424847aa7..603532cc20 100644 --- a/services/web/app/src/infrastructure/ExpressLocals.js +++ b/services/web/app/src/infrastructure/ExpressLocals.js @@ -5,6 +5,7 @@ const _ = require('lodash') const Url = require('url') const Path = require('path') const moment = require('moment') +const pug = require('pug-runtime') const IS_DEV_ENV = ['development', 'test'].includes(process.env.NODE_ENV) @@ -23,6 +24,8 @@ if (!IS_DEV_ENV) { webpackManifest = require(`../../../public/manifest.json`) } +const I18N_HTML_INJECTIONS = new Set() + module.exports = function(webRouter, privateApiRouter, publicApiRouter) { webRouter.use(function(req, res, next) { res.locals.session = req.session @@ -179,6 +182,23 @@ module.exports = function(webRouter, privateApiRouter, publicApiRouter) { webRouter.use(function(req, res, next) { res.locals.translate = function(key, vars, components) { vars = vars || {} + + if (Settings.i18n.checkForHTMLInVars) { + Object.entries(vars).forEach(([field, value]) => { + if (pug.escape(value) !== value) { + const violationsKey = key + field + // do not flood the logs, log one sample per pod + key + field + if (!I18N_HTML_INJECTIONS.has(violationsKey)) { + logger.warn( + { key, field, value }, + 'html content in translations context vars' + ) + I18N_HTML_INJECTIONS.add(violationsKey) + } + } + }) + } + vars.appName = Settings.appName const locale = req.i18n.translate(key, vars) if (components) { diff --git a/services/web/app/src/infrastructure/Translations.js b/services/web/app/src/infrastructure/Translations.js index 96c41ae32f..9f77c00af5 100644 --- a/services/web/app/src/infrastructure/Translations.js +++ b/services/web/app/src/infrastructure/Translations.js @@ -46,7 +46,7 @@ i18n // Disable escaping of interpolated values for backwards compatibility. // We escape the value after it's translated in web, so there's no // security risk - escapeValue: false, + escapeValue: Settings.i18n.escapeHTMLInVars, // Disable nesting in interpolated values, preventing user input // injection via another nested value skipOnVariables: true diff --git a/services/web/config/settings.defaults.coffee b/services/web/config/settings.defaults.coffee index 783f9d518b..c8d4c77714 100644 --- a/services/web/config/settings.defaults.coffee +++ b/services/web/config/settings.defaults.coffee @@ -293,6 +293,8 @@ module.exports = settings = # ------ # i18n: + checkForHTMLInVars: process.env['I18N_CHECK_FOR_HTML_IN_VARS'] == 'true' + escapeHTMLInVars: process.env['I18N_ESCAPE_HTML_IN_VARS'] == 'true' subdomainLang: www: {lngCode:"en", url: siteUrl} defaultLng: "en" diff --git a/services/web/test/unit/src/infrastructure/TranslationsTests.js b/services/web/test/unit/src/infrastructure/TranslationsTests.js index 95efe35d39..a77f31ab97 100644 --- a/services/web/test/unit/src/infrastructure/TranslationsTests.js +++ b/services/web/test/unit/src/infrastructure/TranslationsTests.js @@ -16,6 +16,7 @@ describe('Translations', function() { requires: { 'settings-sharelatex': { i18n: { + escapeHTMLInVars: false, subdomainLang: { www: { lngCode: 'en', url: 'https://www.sharelatex.com' }, fr: { lngCode: 'fr', url: 'https://fr.sharelatex.com' },