From 3cfcb204edd8914fcc7aa9ed62540d11eb55a6a7 Mon Sep 17 00:00:00 2001 From: flvr-odoo Date: Tue, 28 Mar 2023 08:57:49 +0000 Subject: [PATCH] [FIX] web : isolate new tab to external ressources This commit is a security reinforcement. Before this commit: ir.action.act_url was able to handle and redirect to protocols like file:, javascript:, date: ... This is unnecessary in the context of this action. It could potentially be abused by a poorly written custom module It also would not isolate the landing page in case of a new tabs. It means that chrome would still consider the tab to be from the previous domain in case no url was passed and js was executed. This would allow the newly open tab to still make query's to the referrer using the referrer's cookie. While not stricly necessary if we already prevent url that start with "javascript:", it is a nice to have. After this commit: New tabs are not able to access referrer informations or execute javascript interacting with the referrer. Also, it is now impossible to redirect to protocols other than http and https directly from the ir.action.act_url Test update: All new tabs are required to have the "noreferrer" argument Tested an example of an unsupported protocol. closes odoo/odoo#117687 X-original-commit: a020072da17e32fca0bbe6ed02b1afaeec8e8a02 Signed-off-by: Aaron Bohy (aab) --- .../src/webclient/actions/action_service.js | 7 ++++-- .../webclient/actions/url_action_tests.js | 22 ++++++++++++++++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/addons/web/static/src/webclient/actions/action_service.js b/addons/web/static/src/webclient/actions/action_service.js index bb06fbd018b..1936c0775d2 100644 --- a/addons/web/static/src/webclient/actions/action_service.js +++ b/addons/web/static/src/webclient/actions/action_service.js @@ -818,6 +818,9 @@ function makeActionManager(env) { * @param {ActionOptions} options */ function _executeActURLAction(action, options) { + let url = action.url; + if (url && !(url.startsWith('http') || url.startsWith('/'))) + url = '/' + url; if (action.target === "self") { let willUnload = false; const onUnload = () => { @@ -825,13 +828,13 @@ function makeActionManager(env) { }; browser.addEventListener("beforeunload", onUnload); env.services.ui.block(); - browser.location.assign(action.url); + browser.location.assign(url); browser.removeEventListener("beforeunload", onUnload); if (!willUnload) { env.services.ui.unblock(); } } else { - const w = browser.open(action.url, "_blank"); + const w = browser.open(url, "_blank", "noreferrer"); if (!w || w.closed || typeof w.closed === "undefined") { const msg = env._t( "A popup window has been blocked. You may need to change your " + diff --git a/addons/web/static/tests/webclient/actions/url_action_tests.js b/addons/web/static/tests/webclient/actions/url_action_tests.js index 77586ab626b..58819e90d01 100644 --- a/addons/web/static/tests/webclient/actions/url_action_tests.js +++ b/addons/web/static/tests/webclient/actions/url_action_tests.js @@ -57,7 +57,10 @@ QUnit.module("ActionManager", (hooks) => { QUnit.test("execute an 'ir.actions.act_url' action with onClose option", async (assert) => { setupWebClientRegistries(); patchWithCleanup(browser, { - open: () => assert.step("browser open"), + open: (url, target, argument) => { + assert.step("browser open"); + assert.strictEqual(argument, "noreferrer"); + }, }); const env = await makeTestEnv({ serverData }); const options = { @@ -66,4 +69,21 @@ QUnit.module("ActionManager", (hooks) => { await doAction(env, { type: "ir.actions.act_url" }, options); assert.verifySteps(["browser open", "onClose"]); }); + + QUnit.test("execute an 'ir.actions.act_url' action with url javascript:", async (assert) => { + assert.expect(2); + patchWithCleanup(browser.location, { + assign: (url) => { + assert.step(url); + }, + }); + setupWebClientRegistries(); + const env = await makeTestEnv({ serverData }); + await doAction(env, { + type: "ir.actions.act_url", + target: "self", + url: "javascript:alert()", + }); + assert.verifySteps(["/javascript:alert()"]); + }); });