From 6af3ced31eb99942ab1231d6e765faee8f799777 Mon Sep 17 00:00:00 2001 From: Anthony Garland Bot 7 Date: Sat, 27 Sep 2025 23:30:12 +0000 Subject: [PATCH 1/3] Fix XSS vulnerability in inline scripts (Issue #218) - Escape sequences in generated JavaScript to prevent injection. - Add unit test suite with test for script escaping. - Integrate unit tests into CI/CD workflow. Addresses security issue where user-controlled data could inject arbitrary script. --- .github/workflows/regression-tests.yml | 3 +++ tests/run_tests.sh | 16 +++++++++++ tests/test_html_security.py | 37 ++++++++++++++++++++++++++ toyplot/html.py | 21 ++++++++++++--- 4 files changed, 73 insertions(+), 4 deletions(-) create mode 100755 tests/run_tests.sh create mode 100644 tests/test_html_security.py diff --git a/.github/workflows/regression-tests.yml b/.github/workflows/regression-tests.yml index 69b7bf30..e7b9b12b 100644 --- a/.github/workflows/regression-tests.yml +++ b/.github/workflows/regression-tests.yml @@ -35,6 +35,9 @@ jobs: run: | coverage run --source toyplot -m behave --tags=~wip --logging-level INFO --no-logcapture coverage report + - name: Run unit tests + run: | + ./tests/run_tests.sh - name: Upload coverage to Coveralls run: coveralls --service=github env: diff --git a/tests/run_tests.sh b/tests/run_tests.sh new file mode 100755 index 00000000..b4352d5f --- /dev/null +++ b/tests/run_tests.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash +set -euo pipefail + +# run_tests.sh +# Run the Toyplot test suite. +# Requires: Python with unittest module (standard library) + +echo "Running Toyplot test suite..." + +# Change to the project root directory +cd "$(dirname "$0")/.." + +# Run tests using unittest discovery +python -m unittest discover -s tests -p "test_*.py" -v + +echo "All tests passed." \ No newline at end of file diff --git a/tests/test_html_security.py b/tests/test_html_security.py new file mode 100644 index 00000000..6c9e9a14 --- /dev/null +++ b/tests/test_html_security.py @@ -0,0 +1,37 @@ +import unittest +import xml.etree.ElementTree as xml + +import toyplot +import toyplot.html +import toyplot.data +from toyplot.html import RenderContext, _render_javascript # accessing internal for targeted test + + +class TestHTMLSecurity(unittest.TestCase): + + def test_script_closing_tag_escaped(self): + # Create a minimal render context + root = xml.Element("div") + context = RenderContext(scenegraph=None, root=root) + # Simulate a module definition (not strictly necessary, but realistic) + context.define("toyplot/test/module", value={"k": 1}) + # Inject a javascript call with an argument containing the sentinel sequence + malicious = "safe prefix suffix" + context.require( + dependencies=["toyplot/test/module"], + arguments=[malicious], + code="""function(mod, arg){ if(!mod.k){ throw new Error('module fail'); } }""", + ) + # Assign a parent for script injection destination + context = context.copy(parent=root) + _render_javascript(context) + + # Extract script content + scripts = [child.text for child in root.findall("script")] + self.assertTrue(scripts, "No ", combined, "Unescaped found in script block!") + # Escaped form should appear + self.assertIn("<\\/script>", combined, "Escaped not present.") diff --git a/toyplot/html.py b/toyplot/html.py index da29aabf..a6ef1f8b 100644 --- a/toyplot/html.py +++ b/toyplot/html.py @@ -1041,10 +1041,7 @@ def search(name, visited, modules): search(requirement, visited, modules) # Generate the code. - script = """(function() -{ -var modules={}; -""" + script = """(function()\n{\nvar modules={};\n""" # Initialize required modules. for name, (requirements, factory, value) in modules: @@ -1073,6 +1070,22 @@ def search(name, visited, modules): script += """})();""" + # Security Hardening (Issue #218): + # Inline , even if it occurs inside a JavaScript string literal. + # User-controlled data funneled through json.dumps() could therefore inject + # arbitrary script by embedding . + # + # Mitigation: escape every occurrence of inside the script payload + # before inserting it into the DOM. The conventional safe form is <\/script>, + # which the JavaScript engine interprets the same, while the HTML parser does + # not treat it as an end tag. + # + # Note: We intentionally perform a plain string replacement across the entire + # script text. This is safe because it only increases escaping and does not + # alter runtime semantics. Future refactors may move to a data + loader model. + script = script.replace("", "<\\/script>") + # Create the DOM elements. xml.SubElement(context.parent, "script").text = script From aff757c74e4c07613bb5ab5ea84d60f2d73f6723 Mon Sep 17 00:00:00 2001 From: Anthony Garland Bot 7 Date: Sun, 28 Sep 2025 01:47:28 +0000 Subject: [PATCH 2/3] Fix: validate hyperlink schemes to block unsafe URIs (Issue #219) - Only allow http, https, mailto, ftp, or relative URLs in require.hyperlink - Add unit tests for safe and unsafe schemes Addresses security issue where javascript: and data: URIs could be used for injection. --- tests/test_hyperlink_validation.py | 23 +++++++++++++++++++++++ toyplot/require.py | 20 ++++++++++++++++++-- 2 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 tests/test_hyperlink_validation.py diff --git a/tests/test_hyperlink_validation.py b/tests/test_hyperlink_validation.py new file mode 100644 index 00000000..2df8b889 --- /dev/null +++ b/tests/test_hyperlink_validation.py @@ -0,0 +1,23 @@ +import unittest +from toyplot.require import hyperlink + +class TestHyperlinkValidation(unittest.TestCase): + def test_safe_schemes(self): + self.assertEqual(hyperlink("http://example.com"), "http://example.com") + self.assertEqual(hyperlink("https://example.com"), "https://example.com") + self.assertEqual(hyperlink("mailto:user@example.com"), "mailto:user@example.com") + self.assertEqual(hyperlink("ftp://example.com"), "ftp://example.com") + self.assertEqual(hyperlink("/relative/path"), "/relative/path") + self.assertEqual(hyperlink("example.com"), "example.com") + self.assertEqual(hyperlink(None), None) + self.assertEqual(hyperlink(""), "") + + def test_unsafe_schemes(self): + for url in [ + "javascript:alert(1)", + "data:text/html;base64,SGVsbG8sIFdvcmxkIQ==", + "vbscript:msgbox('hi')", + "file:///etc/passwd", + ]: + with self.assertRaises(ValueError, msg=f"Should reject: {url}"): + hyperlink(url) diff --git a/toyplot/require.py b/toyplot/require.py index 49cf4858..a74734f8 100644 --- a/toyplot/require.py +++ b/toyplot/require.py @@ -8,6 +8,7 @@ import numbers import numpy +from urllib.parse import urlparse def instance(value, types): """Raise an exception if a value isn't one of the given type(s).""" @@ -107,9 +108,24 @@ def filename(value): return optional_string(value) +_ALLOWED_URI_SCHEMES = {"", "http", "https", "mailto", "ftp"} + def hyperlink(value): - """Raise an exception if a value isn't a valid string hyperlink, or None.""" - return optional_string(value) + """ + Raise an exception if a value isn't a valid string hyperlink, or None. + Only allows safe URI schemes: http, https, mailto, ftp, or relative URLs (no scheme). + """ + value = optional_string(value) + if value is None: + return value + value = value.strip() + if not value: + return value + parsed = urlparse(value) + scheme = parsed.scheme.lower() + if scheme not in _ALLOWED_URI_SCHEMES: + raise ValueError(f"Disallowed URI scheme: {parsed.scheme}") + return value def as_int(value,precision=None): """Raise an exception if a value cannot be converted to an int, or value From 9782e6c3456b29f9a999f2d88772a1d77d731c61 Mon Sep 17 00:00:00 2001 From: Anthony Garland Bot 7 Date: Sun, 28 Sep 2025 01:49:23 +0000 Subject: [PATCH 3/3] added the test file --- tests/test_require_validation.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 tests/test_require_validation.py diff --git a/tests/test_require_validation.py b/tests/test_require_validation.py new file mode 100644 index 00000000..4ca037eb --- /dev/null +++ b/tests/test_require_validation.py @@ -0,0 +1,29 @@ +import unittest + +import toyplot.require as require + + +class TestHyperlinkValidation(unittest.TestCase): + + def test_hyperlink_accepts_allowed_schemes(self): + self.assertIsNone(require.hyperlink(None)) + self.assertEqual(require.hyperlink(""), "") + self.assertEqual(require.hyperlink("/relative/path"), "/relative/path") + self.assertEqual(require.hyperlink("https://example.com"), "https://example.com") + self.assertEqual(require.hyperlink("HTTP://EXAMPLE.COM"), "HTTP://EXAMPLE.COM") + self.assertEqual(require.hyperlink("mailto:user@example.com"), "mailto:user@example.com") + self.assertEqual(require.hyperlink("ftp://example.com/resource"), "ftp://example.com/resource") + + def test_hyperlink_rejects_disallowed_schemes(self): + with self.assertRaises(ValueError): + require.hyperlink("javascript:alert(1)") + with self.assertRaises(ValueError): + require.hyperlink("data:text/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0PiIp") + with self.assertRaises(ValueError): + require.hyperlink("vbscript:msgbox('hi')") + with self.assertRaises(ValueError): + require.hyperlink(" javascript:alert(1)") + + +if __name__ == "__main__": + unittest.main()