2032951 - Test coverage for canvas tainting and MV3

Status: RESOLVED FIXED

(WebExtensions :: General, task, P3)

Description

[Rob Wu [:robwu]]

In https://bugzilla.mozilla.org/show_bug.cgi?id=1758306#c2 I mentioned that I expected <all_urls> to not be supported to read from a canvas in content scripts in MV3, consistent with how host permission support was dropped for fetch (bug 1578405).

When I wrote a test, expecting it to fail due to bug 1758306, I was surprised to see the test already passing without any fix to bug 1758306. But since the test was written anyway, I'll attach it as a patch, then we can have test coverage for the scenario and figure out whether we want to change the behavior.

Comment 1

[Rob Wu [:robwu]]

Attachment: Bug 2032951 - Add test for canvas tainting and MV3

Comment 2

[Rob Wu [:robwu]]

(In reply to Rob Wu [:robwu] from comment #0)

When I wrote a test, expecting it to fail due to bug 1758306, I was surprised to see the test already passing without any fix to bug 1758306.

Ah I see what is going on.. the test function that tells whether the request suceeded returned true unconditionally. The test did in fact prove that bug 1758306 failed without the fix.

The test coverage added here also provides coverage for the scenario in bug 1957794.

Comment 3

[Rob Wu [:robwu]]

(In reply to Rob Wu [:robwu] from comment #0)

In https://bugzilla.mozilla.org/show_bug.cgi?id=1758306#c2 I mentioned that I expected <all_urls> to not be supported to read from a canvas in content scripts in MV3, consistent with how host permission support was dropped for fetch (bug 1578405).

figure out whether we want to change the behavior.

I discussed this with :zombie and :rpl.

Extension reading from tainted canvas was implemented in bug 1318565 and bug 1484980.

For background scripts, requiring <all_urls> is very intentional, because a canvas has only a tainted flag, without storing the origins that made it tainted.

For content scripts, there are use cases seeking the ability to read from canvas.
A reason for restricting access is to reduce cross-origin data reads to the content process. For fetch this was done in bug 1687762, which Chrome also did in preparation for their enforcement of Site isolation ( https://new.chromium.org/Home/chromium-security/extension-content-script-fetches/ ). Since we already support reading from tainted canvas, we are going to keep the behavior. However, we are not going to document this as officially supported; we want to be able to deprecate / remove this functionality if there is a reason to do so, for example if improved security hardening work makes it infeasible to continue support for this functionality.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/800addec4366
https://hg.mozilla.org/integration/autoland/rev/a420128eeedc
Add test for canvas tainting and MV3 r=zombie

Comment 5

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/a420128eeedc