From 70b3d21da2585c5a214a7b1292f5a0f5edd92f5e Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Thu, 30 Jul 2020 19:20:16 -0400 Subject: [PATCH 1/5] Fix clobbering of overridden canvas methods The method may be overridden again after we override, as is done in the `hidpi-canvas-polyfill`. So if we restore the method to the original as we see it, we'll wipe out the polyfill. Fixes #2657, #1226 --- src/js/contentscripts/fingerprinting.js | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/js/contentscripts/fingerprinting.js b/src/js/contentscripts/fingerprinting.js index 28cdb4641b..4af10bb9dc 100644 --- a/src/js/contentscripts/fingerprinting.js +++ b/src/js/contentscripts/fingerprinting.js @@ -204,14 +204,19 @@ function getFpPageScript() { ); item.obj[item.propName] = (function (orig) { + // set to true after the first write, if the method is not + // restorable. Happens if another library also overwrites + // this method. + var skipMonitoring = false; function wrapped() { var args = arguments; if (is_canvas_write) { // to avoid false positives, - // bail if the text being written is too short - if (!args[0] || args[0].length < 5) { + // bail if the text being written is too short, + // of if we've already sent a monitoring payload + if (skipMonitoring || !args[0] || args[0].length < 5) { return orig.apply(this, args); } } @@ -237,7 +242,13 @@ function getFpPageScript() { // optimization: one canvas write is enough, // restore original write method // to this CanvasRenderingContext2D object instance - this[item.propName] = orig; + // Careful! Only restorable if we haven't already been replaced + // by another lib, such as the hidpi polyfill + if (this[item.propName] === wrapped) { + this[item.propName] = orig; + } else { + skipMonitoring = true; + } } return orig.apply(this, args); From 56fc2c7f598e0bf6f21493257461c051a830367e Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Fri, 31 Jul 2020 11:00:50 -0400 Subject: [PATCH 2/5] Add canvas polyfill retention test --- .../fingerprinting_canvas_hidpi_test.py | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 tests/selenium/fingerprinting_canvas_hidpi_test.py diff --git a/tests/selenium/fingerprinting_canvas_hidpi_test.py b/tests/selenium/fingerprinting_canvas_hidpi_test.py new file mode 100644 index 0000000000..ff5fece2f3 --- /dev/null +++ b/tests/selenium/fingerprinting_canvas_hidpi_test.py @@ -0,0 +1,41 @@ +#!/usr/bin/env python +# -*- coding: UTF-8 -*- + +import unittest + +import pbtest + +from pbtest import retry_until + + +class FingerprintingTest(pbtest.PBSeleniumTest): + """Tests to make sure we don't restore a native canvas function that has been overridden.""" + + # Returns true if function is native + def detected_native_function(self): + return self.js(""" + const canvas = document.getElementById("writetome"); + const ctx = canvas.getContext("2d"); + return ctx.fillText.toString().indexOf("[native code]") !== -1; + """) + + # TODO can fail because our content script runs too late: https://crbug.com/478183 + @pbtest.repeat_if_failed(3) + def test_canvas_fingerprinting_detection(self): + FIXTURE_URL = ( + "https://efforg.github.io/privacybadger-test-fixtures/html/" + "fingerprinting_canvas_hidpi.html" + ) + + # visit the page + self.load_url(FIXTURE_URL) + + # check that we did not restore the native function (should be hipdi polyfill) + self.assertFalse( + self.detected_native_function(), + "Canvas context fillText is not native version (polyfill has been retained)." + ) + + +if __name__ == "__main__": + unittest.main() From 3e0b0c359bfac1de317b8f30803846384d474f4e Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Fri, 31 Jul 2020 13:39:35 -0400 Subject: [PATCH 3/5] Add how to run selenium tests in Chrome on MacOS --- doc/tests.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/tests.md b/doc/tests.md index 1c99a0b191..b04af050fb 100644 --- a/doc/tests.md +++ b/doc/tests.md @@ -50,9 +50,11 @@ the code below. This should take several minutes. $ BROWSER=chrome pytest -v ``` -macOS users may need to provide the full path to the browser application folder. For example, to run tests on macOS in Firefox: +macOS users may need to provide the full path to the browser application folder. For example, to run tests on macOS: ```bash $ BROWSER=/Applications/Firefox.app/Contents/MacOS/firefox-bin pytest -v +# or +$ BROWSER=/Applications/Google\ Chrome.app/Contents/MacOS/Google\ Chrome pytest -v ``` For more information, see our Travis CI [setup](/scripts/setup_travis.sh) and From 165d12b55c8519c8555775dc35331885b9564b98 Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Fri, 31 Jul 2020 17:59:22 -0400 Subject: [PATCH 4/5] fix browser detection in test Should be able to figure out that "Google Chrome" is chrome --- tests/selenium/pbtest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/selenium/pbtest.py b/tests/selenium/pbtest.py index 88b72b421c..cc18a0f243 100644 --- a/tests/selenium/pbtest.py +++ b/tests/selenium/pbtest.py @@ -44,7 +44,7 @@ def unix_which(command, silent=False): def get_browser_type(string): for t in BROWSER_TYPES: - if t in string: + if t in string.lower(): return t raise ValueError("couldn't get browser type from %s" % string) From aef17af082b1a134053fec9912e9245874e1d846 Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Fri, 31 Jul 2020 17:59:37 -0400 Subject: [PATCH 5/5] Address @ghostwords comments and combine canvas suites --- src/js/contentscripts/fingerprinting.js | 6 +-- .../fingerprinting_canvas_hidpi_test.py | 41 ------------------- tests/selenium/fingerprinting_test.py | 25 +++++++++++ 3 files changed, 28 insertions(+), 44 deletions(-) delete mode 100644 tests/selenium/fingerprinting_canvas_hidpi_test.py diff --git a/src/js/contentscripts/fingerprinting.js b/src/js/contentscripts/fingerprinting.js index 4af10bb9dc..b6dbb2ea0e 100644 --- a/src/js/contentscripts/fingerprinting.js +++ b/src/js/contentscripts/fingerprinting.js @@ -207,7 +207,7 @@ function getFpPageScript() { // set to true after the first write, if the method is not // restorable. Happens if another library also overwrites // this method. - var skipMonitoring = false; + var skip_monitoring = false; function wrapped() { var args = arguments; @@ -216,7 +216,7 @@ function getFpPageScript() { // to avoid false positives, // bail if the text being written is too short, // of if we've already sent a monitoring payload - if (skipMonitoring || !args[0] || args[0].length < 5) { + if (skip_monitoring || !args[0] || args[0].length < 5) { return orig.apply(this, args); } } @@ -247,7 +247,7 @@ function getFpPageScript() { if (this[item.propName] === wrapped) { this[item.propName] = orig; } else { - skipMonitoring = true; + skip_monitoring = true; } } diff --git a/tests/selenium/fingerprinting_canvas_hidpi_test.py b/tests/selenium/fingerprinting_canvas_hidpi_test.py deleted file mode 100644 index ff5fece2f3..0000000000 --- a/tests/selenium/fingerprinting_canvas_hidpi_test.py +++ /dev/null @@ -1,41 +0,0 @@ -#!/usr/bin/env python -# -*- coding: UTF-8 -*- - -import unittest - -import pbtest - -from pbtest import retry_until - - -class FingerprintingTest(pbtest.PBSeleniumTest): - """Tests to make sure we don't restore a native canvas function that has been overridden.""" - - # Returns true if function is native - def detected_native_function(self): - return self.js(""" - const canvas = document.getElementById("writetome"); - const ctx = canvas.getContext("2d"); - return ctx.fillText.toString().indexOf("[native code]") !== -1; - """) - - # TODO can fail because our content script runs too late: https://crbug.com/478183 - @pbtest.repeat_if_failed(3) - def test_canvas_fingerprinting_detection(self): - FIXTURE_URL = ( - "https://efforg.github.io/privacybadger-test-fixtures/html/" - "fingerprinting_canvas_hidpi.html" - ) - - # visit the page - self.load_url(FIXTURE_URL) - - # check that we did not restore the native function (should be hipdi polyfill) - self.assertFalse( - self.detected_native_function(), - "Canvas context fillText is not native version (polyfill has been retained)." - ) - - -if __name__ == "__main__": - unittest.main() diff --git a/tests/selenium/fingerprinting_test.py b/tests/selenium/fingerprinting_test.py index 657999937a..6e1d22d09c 100644 --- a/tests/selenium/fingerprinting_test.py +++ b/tests/selenium/fingerprinting_test.py @@ -36,6 +36,13 @@ def detected_tracking(self, domain, page_url): map[tracker_origin].indexOf(site_origin) != -1 );""".format(domain, page_url)) + def get_fillText_source(self): + return self.js(""" + const canvas = document.getElementById("writetome"); + const ctx = canvas.getContext("2d"); + return ctx.fillText.toString(); + """) + # TODO can fail because our content script runs too late: https://crbug.com/478183 @pbtest.repeat_if_failed(3) def test_canvas_fingerprinting_detection(self): @@ -69,6 +76,24 @@ def test_canvas_fingerprinting_detection(self): "Canvas fingerprinting resource was detected as a fingerprinter." ) + # Privacy Badger overrides a few functions on canvas contexts to check for fingerprinting. + # In previous versions, it would restore the native function after a single call. Unfortunately, + # this would wipe out polyfills that had also overridden the same functions, such as the very + # popular "hidpi-canvas-polyfill". + def test_canvas_polyfill_clobbering(self): + FIXTURE_URL = ( + "https://efforg.github.io/privacybadger-test-fixtures/html/" + "fingerprinting_canvas_hidpi.html" + ) + + # visit the page + self.load_url(FIXTURE_URL) + + # check that we did not restore the native function (should be hipdi polyfill) + self.assertNotIn("[native code]", self.get_fillText_source(), + "Canvas context fillText is not native version (polyfill has been retained)." + ) + if __name__ == "__main__": unittest.main()