Merge pull request #3345 from overleaf/jpa-i18n-escape-vars

[misc] i18n escape vars

GitOrigin-RevId: 5b0a8ca7229f3817c823b126203c8597f7fd0913

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) {

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

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"

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' },