diff --git a/CHANGELOG.md b/CHANGELOG.md index fad35b7..7f3ca2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Precision + +- A Django template file the analyzer cannot read, because of its permissions, no longer counts as escaping everything it renders: it read the file as empty text, and an empty template escapes. The flow through `render_to_string` stays `xss`, and a template extending or including the unreadable one is not established to escape either, as for a template the analyzer cannot find. An unreadable file was already listed among the templates the project names but the run could not read. + ## 0.17.0 (2026-09-28) ### Plugins diff --git a/docs/usage.md b/docs/usage.md index a26cd13..e3f866e 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -156,9 +156,9 @@ It establishes escaping only when all of the following hold: - No Python file of the project sets `autoescape` to `False`. The analyzer never assumes escaping where it cannot read the template. That covers a -template named by an expression, a template found through a `DIRS` entry not named -`templates`, one shipped by an installed package, one rendered through `get_template`, -and Flask's Jinja2 templates. The flow is then reported as before. A single-file check +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 +rendered through `get_template`, and Flask's Jinja2 templates. The flow is then reported as before. A single-file check reads no template. ### Dangerous API usage diff --git a/src/coretrace_python/taint/templates.py b/src/coretrace_python/taint/templates.py index 10bc65a..a41ed57 100644 --- a/src/coretrace_python/taint/templates.py +++ b/src/coretrace_python/taint/templates.py @@ -15,7 +15,8 @@ Templates are found under the project's ``templates`` directories, by their path below one. A template the engine cannot find — named by an expression, under a ``DIRS`` entry -of another name, shipped by an installed package — is never assumed to escape. +of another name, shipped by an installed package — or cannot read is never assumed to +escape. A template applying a filter of Django's own libraries calls the function behind it whenever it is rendered, though no Python call names that function: ``{{ bio|striptags }}`` @@ -149,7 +150,7 @@ def escaped_templates(root: Path) -> frozenset[str]: local: dict[str, bool] = {} requires: dict[str, set[str]] = {} for name, paths in found.items(): - verdicts = [_inspect(_read(path)) for path in paths] + verdicts = [_verdict(path) for path in paths] local[name] = all(safe for safe, _ in verdicts) requires[name] = {needed for _, needs in verdicts for needed in needs} escaped = {name for name, safe in local.items() if safe} @@ -171,6 +172,14 @@ def _read(path: Path) -> str: return _text(path) or "" +def _verdict(path: Path) -> tuple[bool, set[str]]: + """Whether one template file escapes all it prints, and what it needs: a file the + engine cannot read is not established to escape, like a template it cannot find.""" + + text = _text(path) + return (False, set()) if text is None else _inspect(text) + + def _text(path: Path) -> str | None: """The text of ``path``, or None when it cannot be read.""" diff --git a/tests/test_unreadable_templates.py b/tests/test_unreadable_templates.py new file mode 100644 index 0000000..09273a3 --- /dev/null +++ b/tests/test_unreadable_templates.py @@ -0,0 +1,70 @@ +"""A template file the engine cannot read proves nothing about escaping. + +The analyzer establishes that a Django template escapes everything it renders only from +its text. A template file it cannot read, because of its permissions, is not assumed to +escape, no more than a template it cannot find: the flow through ``render_to_string`` +stays ``xss``, and a template extending or including the unreadable one is not +established either. +""" + +from __future__ import annotations + +import os +from pathlib import Path + +import pytest + +from coretrace_python import engine +from coretrace_python.taint.templates import escaped_templates + +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('app/page.html', {'name': request.GET['name']}))\n" +) +ESCAPING = "

{{ name }}

\n" + +pytestmark = pytest.mark.skipif(os.name != "posix" or os.geteuid() == 0, reason="needs a file its owner cannot read") + + +def write(root: Path, files: dict[str, str]) -> None: + for relative, text in files.items(): + (root / relative).parent.mkdir(parents=True, exist_ok=True) + (root / relative).write_text(text, encoding="utf-8") + + +def unreadable(path: Path) -> None: + path.chmod(0) + + +@pytest.fixture +def readable_again(tmp_path: Path): # type: ignore[no-untyped-def] + yield + for path in tmp_path.rglob("*"): + if path.is_file(): + path.chmod(0o644) + + +def test_a_template_file_that_cannot_be_read_is_not_escaped(tmp_path: Path, readable_again: None) -> None: + write(tmp_path, {"app/templates/app/page.html": ESCAPING, "app/templates/app/other.html": ESCAPING}) + unreadable(tmp_path / "app/templates/app/page.html") + + assert escaped_templates(tmp_path) == frozenset({"app/other.html"}) + + +def test_rendering_a_template_that_cannot_be_read_stays_cross_site_scripting(tmp_path: Path, readable_again: None) -> None: + write(tmp_path, {"app/views.py": VIEW, "app/templates/app/page.html": ESCAPING}) + unreadable(tmp_path / "app/templates/app/page.html") + + findings = engine.analyze_project(tmp_path, [engine.BUNDLED_PLUGINS]).findings + + assert [f.span.start_line for f in findings if f.rule_id == "xss"] == [5] + + +@pytest.mark.parametrize("tag", ["{% extends 'app/base.html' %}", "{% include 'app/base.html' %}"]) +def test_a_template_needing_one_that_cannot_be_read_is_not_escaped(tmp_path: Path, readable_again: None, tag: str) -> None: + write(tmp_path, {"app/templates/app/page.html": tag + ESCAPING, "app/templates/app/base.html": ESCAPING}) + unreadable(tmp_path / "app/templates/app/base.html") + + assert escaped_templates(tmp_path) == frozenset()