diff --git a/docs/usage.md b/docs/usage.md index 5ea6847..98d85a7 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -197,6 +197,11 @@ It establishes escaping only when all of the following hold: or an attribute name. - No Python file of the project sets `autoescape` to `False`. +Start tags and their attributes are read as a browser's HTML tokenizer reads them, in time +linear in the size of the template. A tag, or a quoted attribute value, a template leaves +open at its end is checked too: once the template is included, it runs into the markup +that follows, the value staying quoted until its quote closes. + The analyzer never assumes escaping where it cannot read the template. That covers a template file it cannot read, a template named by an expression, a template found through a `DIRS` entry not named `templates`, one shipped by an installed package, one diff --git a/src/coretrace_python/taint/templates.py b/src/coretrace_python/taint/templates.py index 1072144..ea54bf9 100644 --- a/src/coretrace_python/taint/templates.py +++ b/src/coretrace_python/taint/templates.py @@ -140,8 +140,20 @@ # fixed: no scheme such as ``javascript:`` can follow. _SAFE_URL_PREFIX = re.compile(r"(?:/(?!/)|\.{1,2}/|[#?]|https?://[^/?#\x00\x01]+[/?#])") _QUOTED = r"\"[^\"]*\"|'[^']*'" -_ATTRIBUTE = re.compile(rf"([^\s\"'>/=]+)(?:\s*=\s*({_QUOTED}|[^\s\"'=<>`]+))?") -_START_TAG = re.compile(rf"<([A-Za-z][^\s/>]*)((?:\s*[^\s\"'>/=]+(?:\s*=\s*(?:{_QUOTED}|[^\s\"'=<>`]+))?)*)\s*/?>") +# A start tag read as the HTML tokenizer reads one: the tag name runs to whitespace, ``/`` +# or ``>``; between attributes, whitespace and a stray ``/`` are skipped; an attribute +# name, which may begin with ``=``, runs to whitespace, ``/``, ``>`` or ``=``; a value +# follows ``=``, quoted, or running to whitespace or ``>``. Each token is told apart by +# its first character, so matching is linear, where splitting runs between attributes +# took exponential time (#263). A tag the text ends in before its ``>`` still matches, +# to the end: a template may be included before more markup, which the tag then runs +# into, so its attributes are checked like any other's. A value that opens a quote +# stays quoted to the matching quote or, for the same reason, to the end of the text: it +# is never read again as an unquoted value. +_ATTRIBUTE_NAME = r"(?:=[^\s/>=]*+|[^\s/>=]++)" +_ATTRIBUTE_VALUE = r"(?:\"[^\"]*+(?:\"|\Z)|'[^']*+(?:'|\Z)|[^\s>]*+)" +_ATTRIBUTE = re.compile(rf"({_ATTRIBUTE_NAME})(?:\s*+=\s*+({_ATTRIBUTE_VALUE}))?") +_START_TAG = re.compile(rf"<([A-Za-z][^\s/>]*+)((?:[\s/]++|{_ATTRIBUTE_NAME}(?:\s*+=\s*+{_ATTRIBUTE_VALUE})?)*+)>?") _RAW_TEXT = re.compile(r"<(script|style)\b[^>]*>(.*?)", re.DOTALL | re.IGNORECASE) _COMMENT = re.compile(r"", re.DOTALL) _END_TAG = re.compile(r"]*>") diff --git a/tests/test_template_tag_backtracking.py b/tests/test_template_tag_backtracking.py new file mode 100644 index 0000000..c40599a --- /dev/null +++ b/tests/test_template_tag_backtracking.py @@ -0,0 +1,193 @@ +"""Acceptance tests for issue #263: the escaping inspection of a template ends, whatever +the file under a ``templates`` directory holds. + +``_START_TAG`` could split a run of attribute-name characters, or of an unquoted value, +between consecutive attributes in exponentially many ways, and tried them all when no +``>`` followed: a few dozen characters, in a text template or a binary file decoded with +replacement characters, stopped the analysis. Each inspection runs here in a subprocess +with a timeout, so that one that does not end fails instead of hanging the suite. The +inspection still refuses to establish escaping where a variable stands in a dangerous +attribute, however long what comes before it. + +Start tags are read as the HTML tokenizer reads them. A tag a template leaves open at its +end runs, once the template is included, into the markup that follows: its attributes +are checked like any other's. +""" + +from __future__ import annotations + +import json +import subprocess +import sys +from pathlib import Path + +import pytest + +# A run long enough that no exponential inspection would ever end; a linear one takes +# milliseconds. +RUN = 5000 +TIMEOUT = 60 + +ESCAPED_TEMPLATES = ( + "import json, sys\n" + "from pathlib import Path\n" + "from coretrace_python.taint.templates import escaped_templates\n" + "print(json.dumps(sorted(escaped_templates(Path(sys.argv[1])))))\n" +) + + +def escaped(tmp_path: Path, files: dict[str, str | bytes]) -> list[str]: + """The names of the templates ``escaped_templates`` establishes for a project of + ``files``, computed in a subprocess with a timeout.""" + + root = tmp_path / "project" + for relative, content in files.items(): + path = root / relative + path.parent.mkdir(parents=True, exist_ok=True) + if isinstance(content, bytes): + path.write_bytes(content) + else: + path.write_text(content, encoding="utf-8") + try: + completed = subprocess.run( + [sys.executable, "-c", ESCAPED_TEMPLATES, str(root)], + capture_output=True, + text=True, + check=True, + timeout=TIMEOUT, + ) + except subprocess.TimeoutExpired: + pytest.fail(f"the inspection did not end within {TIMEOUT} s") + names: list[str] = json.loads(completed.stdout) + return names + + +@pytest.mark.parametrize( + "template", + [ + "

", + " None: + # No variable is printed, so the template escapes. + assert escaped(tmp_path, {"app/templates/page.html": template}) == ["page.html"] + + +def test_a_binary_file_under_templates_does_not_stop_the_inspection(tmp_path: Path) -> None: + # Invalid UTF-8 decodes to replacement characters, which may form attribute names. + image = b"\x89PNG\r\n\x1a\n" + b"{{ name }}

", + } + + assert "page.html" in escaped(tmp_path, files) + + +@pytest.mark.parametrize( + "template", + [ + "x", + "
x
", + "x', + "x", + "
x
", + ], + ids=["event-handler", "unquoted-value", "url-attribute", "handler-right-after-a-quoted-value", "attribute-name"], +) +def test_a_variable_in_a_dangerous_attribute_after_a_long_run_is_still_refused(tmp_path: Path, template: str) -> None: + assert escaped(tmp_path, {"app/templates/page.html": template}) == [] + + +def test_a_variable_in_a_quoted_attribute_after_a_long_run_is_still_escaped(tmp_path: Path) -> None: + template = "x' + + assert escaped(tmp_path, {"app/templates/page.html": template}) == ["page.html"] + + +@pytest.mark.parametrize( + ("template", "names"), + [ + ("

Hello

Hello

Hello

Hello

None: + assert escaped(tmp_path, {"app/templates/page.html": template}) == names + + +@pytest.mark.parametrize( + ("template", "names"), + [ + # After a stray ``/``, the variable starts an attribute name. + ("x", []), + # An attribute name may begin with ``=``: here ``="`` and the variable. + ('
x
', []), + # ``=`` is an attribute of its own, and ``title`` a quoted one after it. + ('
x
', ["page.html"]), + ], + ids=["after-a-slash", "name-beginning-with-equals", "equals-then-quoted-title"], +) +def test_attributes_are_read_as_a_browser_reads_them(tmp_path: Path, template: str, names: list[str]) -> None: + assert escaped(tmp_path, {"app/templates/page.html": template}) == names + + +# A value that opens a quote stays quoted to the matching quote or, left open, to the end +# of the template: once included, the markup that follows continues it. Read again as an +# unquoted value, ``"/*`` ended at the first space and ``title='…'`` read as a separate, +# safe attribute, while the variable sits in the handler. +@pytest.mark.parametrize( + ("template", "names"), + [ + ("", []), + (" None: + assert escaped(tmp_path, {"app/templates/page.html": template}) == names + + +INCLUDING_VIEW = ( + "from django.http import HttpRequest, HttpResponse\n" + "from django.template.loader import render_to_string\n\n" + "def page(request: HttpRequest):\n" + " return HttpResponse(render_to_string('page.html', {'name': request.GET['name']}))\n" +) + + +def test_a_handler_whose_quote_closes_in_the_including_template_is_cross_site_scripting(tmp_path: Path) -> None: + # Rendered, the handler is ``/* title='<…'>{…}">``: a name such as ``*/alert(1);//`` + # runs in it. + files: dict[str, str | bytes] = { + "app/views.py": INCLUDING_VIEW, + "app/templates/fragment.html": "\n", + "app/templates/page.html": "{% include 'fragment.html' %}\">click\n", + } + + assert escaped(tmp_path, files) == [] + root = tmp_path / "project" + try: + completed = subprocess.run( + [sys.executable, "-m", "coretrace_python", "--check", str(root), "--format", "json"], + capture_output=True, + text=True, + check=False, + timeout=TIMEOUT, + ) + except subprocess.TimeoutExpired: + pytest.fail(f"the analysis did not end within {TIMEOUT} s") + findings = json.loads(completed.stdout)["findings"] + assert [(f["location"]["path"], f["location"]["line"]) for f in findings if f["rule_id"] == "xss"] == [ + ("app/views.py", 5) + ]