From 78792b871bd09084915757bcc9bddd042ce84fa3 Mon Sep 17 00:00:00 2001 From: Artyom Zakharchenko Date: Thu, 10 Aug 2023 17:37:28 +0100 Subject: [PATCH] chore: add warn log for sanitisation for overridden config --- ilc/server/app.js | 1 + ilc/server/tailor/parse-override-config.js | 18 ++++++++++++------ .../tailor/parse-override-config.spec.js | 7 ++++++- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/ilc/server/app.js b/ilc/server/app.js index 33f96af7..e8b8a118 100644 --- a/ilc/server/app.js +++ b/ilc/server/app.js @@ -119,6 +119,7 @@ module.exports = (registryService, pluginManager, context) => { const overrideConfigs = parseOverrideConfig( req.headers.cookie, registryConfig.settings.overrideConfigTrustedOrigins, + logger, ); // Excluding LDE related transactions from NewRelic if (overrideConfigs !== null) { diff --git a/ilc/server/tailor/parse-override-config.js b/ilc/server/tailor/parse-override-config.js index 6c614bad..5930fb22 100644 --- a/ilc/server/tailor/parse-override-config.js +++ b/ilc/server/tailor/parse-override-config.js @@ -42,17 +42,23 @@ const isTrustedOrigin = (link, trustedOrigins) => { }); }; -const sanitizeSpoofedLinks = (obj, trustedOrigins) => { +const sanitizeSpoofedLinks = (obj, trustedOrigins, logger) => { Object.entries(obj).forEach(([key, value]) => { if (_.isPlainObject(value)) { - sanitizeSpoofedLinks(value, trustedOrigins); + sanitizeSpoofedLinks(value, trustedOrigins, logger); } else if (typeof value === 'string' && isUrl(value.trim())) { - !isPrivateNetwork(value) && !isTrustedOrigin(value, trustedOrigins) && delete obj[key]; + if (!isPrivateNetwork(value) && !isTrustedOrigin(value, trustedOrigins)) { + if (logger) { + logger.warn(`Sanitized untrusted url from override config. key = ${key}, value = ${value}`); + } + + delete obj[key]; + } } }); }; -module.exports = (cookie, trustedOrigins) => { +module.exports = (cookie, trustedOrigins, logger) => { try { let overrideConfig = typeof cookie === 'string' && cookie.split(';').find((n) => n.trim().startsWith('ILC-overrideConfig')); @@ -75,11 +81,11 @@ module.exports = (cookie, trustedOrigins) => { typeof trustedOrigins === 'string' && trustedOrigins.split(',').map((n) => n.trim()); if (overrideConfig.apps) { - sanitizeSpoofedLinks(overrideConfig.apps, parsedTrustedOrigin); + sanitizeSpoofedLinks(overrideConfig.apps, parsedTrustedOrigin, logger); } if (overrideConfig.sharedLibs) { - sanitizeSpoofedLinks(overrideConfig.sharedLibs, parsedTrustedOrigin); + sanitizeSpoofedLinks(overrideConfig.sharedLibs, parsedTrustedOrigin, logger); } } diff --git a/ilc/server/tailor/parse-override-config.spec.js b/ilc/server/tailor/parse-override-config.spec.js index 1fd9aef9..7db7f269 100644 --- a/ilc/server/tailor/parse-override-config.spec.js +++ b/ilc/server/tailor/parse-override-config.spec.js @@ -1,4 +1,5 @@ import { expect } from 'chai'; +import sinon from 'sinon'; const LZUTF8 = require('lzutf8'); const parseOverrideConfig = require('./parse-override-config'); @@ -283,8 +284,12 @@ describe('overrideConfig', () => { it('should sanitize domain names', async () => { const ip = 'foo.com'; const exampleCookies = getExampleCookies(ip); + const logger = { + warn: sinon.spy(), + }; - expect(parseOverrideConfig(exampleCookies)).deep.equal(getSanitizedObject()); + expect(parseOverrideConfig(exampleCookies, undefined, logger)).deep.equal(getSanitizedObject()); + expect(logger.warn.called).to.be.true; }); it('should sanitize url w/ spaces', async () => {