[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) <aab@odoo.com>
This commit is contained in:
flvr-odoo
2023-04-05 08:38:30 +02:00
parent 0d4a89ac9d
commit 3cfcb204ed
2 changed files with 26 additions and 3 deletions
@@ -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 " +
@@ -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()"]);
});
});