Rich-text fields end up rendered with |safe, and the reasoning is always the
same: the HTML comes from our own editor, typed by our own staff, in our own
back-office. There is no untrusted input.
The reasoning is wrong, and the correction is not “your staff might be
malicious”. It is that |safe makes one compromised account into full
JavaScript execution on every page of the site, with the session cookies of
every visitor and every other administrator in scope. A phished password becomes
a persistent XSS, and the difference between “annoying” and “catastrophic” is a
password.
This is the richtext filter that renders the article you are reading — the
sanitisation layer between the Quill editor and the page, on
altius-group.ch. It is four hundred lines, and almost
none of it is the sanitiser itself.
The allow-list, and the thing it cannot say
ALLOWED_TAGS = {
"h1", "h2", "h3", "h4", "h5", "h6", "p", "br", "hr", "span", "div",
"b", "strong", "i", "em", "u", "s", "strike", "sub", "sup",
"blockquote", "pre", "code", "ul", "ol", "li", "a", "img", "iframe",
"table", "thead", "tbody", "tr", "th", "td",
}
ALLOWED_ATTRIBUTES = {
"a": {"href", "title", "target"}, # `rel` géré par link_rel (nh3)
"img": {"src", "alt", "title", "width", "height", "class"},
"iframe": {"src", "width", "height", "title", "class",
"allow", "allowfullscreen", "frameborder"},
"span": {"style", "class"},
...
}
An allow-list, not a deny-list — nh3 (the Python binding for Rust’s ammonia)
takes the set of tags you permit and discards everything else. Deny-lists lose:
the set of dangerous constructs grows with every browser release, and yours was
written once.
The set is derived from what Quill actually produces, not from what HTML offers. That is the correct source, and it makes the list auditable: a tag that appears in stored content and is not in this set arrived from somewhere other than the editor, which is itself worth knowing.
But look at iframe in that list. Allowing the tag says an iframe may exist.
It says nothing about where it may point — and an unrestricted iframe on your
own domain is a phishing page with your padlock and your address bar:
"""
``<iframe>`` est autorisé **uniquement** vers les hébergeurs vidéo de confiance :
un iframe libre laisserait un compte d'équipe compromis encadrer n'importe quel
site — hameçonnage crédible sous notre domaine.
"""
Same problem with class on an image: allowing the attribute lets anyone apply
any class in the site’s stylesheet to any image.
Neither constraint is about tags or attribute names. Both are about values, and that is a third level of allow-list that most sanitiser configurations never reach.
The attribute filter
nh3 provides the hook:
def _attribute_filter(tag, attribute, value):
"""Dernier filtre, appliqué par nh3 attribut par attribut.
Sert à ce que la liste blanche porte sur la VALEUR d'un attribut, ce que la
liste de balises ne sait pas exprimer : l'hôte d'un `src` d'iframe, et les
classes qu'on accepte de voir sur une image."""
if tag == "iframe" and attribute == "src" and not is_trusted_embed(value):
return None
if attribute == "class" and tag in ("img", "span"):
kept = [c for c in (value or "").split() if c in MEDIA_CLASSES]
return " ".join(kept) or None
if tag == "li" and attribute == "class":
kept = [c for c in (value or "").split() if c in INDENT_CLASSES]
return " ".join(kept) or None
return value
Return the value to keep it, a different value to rewrite it, None to drop the
attribute.
Classes are filtered one by one, not accepted or rejected as a string. A
class attribute is a list, and "ag-w-half ag-nav-admin" must lose exactly one
of its two entries. Whole-string matching would either drop a legitimate layout
class or keep an injected one.
None rather than "". An image with no recognised class does not keep an
empty class="" in the markup — the attribute goes away entirely.
And the host check:
VIDEO_HOSTS = frozenset({
"www.youtube.com", "youtube.com",
"www.youtube-nocookie.com", "youtube-nocookie.com",
"player.vimeo.com",
})
def is_trusted_embed(url):
try:
parsed = urlparse((url or "").strip())
except ValueError:
return False
return parsed.scheme == "https" and parsed.hostname in VIDEO_HOSTS
# Comparés à l'hôte EXACT (ou à un sous-domaine), jamais par « contient » —
# « youtube.com.pirate.tld » passerait.
parsed.hostname in VIDEO_HOSTS — set membership on the parsed hostname. Not
"youtube.com" in url, which matches https://youtube.com.pirate.tld/embed/x
and https://evil.tld/?ref=youtube.com. This is the single most common URL
allow-list bug and it is one line away from being correct.
urlparse first, then compare the hostname field. Never substring matching on
a URL, ever, for any purpose.
scheme == "https" explicitly: an http embed would be blocked as mixed content
on a secure page anyway, so permitting it adds a failure mode and no capability.
The wrapper that bypassed the filter
Here is a real bug from this file, and it is the shape that value-level allow-lists tend to fail in.
# `span` autant que `img` : Quill n'applique pas une classe en ligne sur
# l'image elle-même, il l'ENVELOPPE dans un `<span>` qui porte la classe.
# Sans filtrer aussi le span, l'attribut passait sans contrôle — un compte
# d'équipe compromis pouvait y poser n'importe quelle classe de la feuille
# de style du site, ce que le filtrage de `img` était précisément là pour
# empêcher.
if attribute == "class" and tag in ("img", "span"):
The filter was written for img, because the class is a property of the image.
Quill does not put it there. For an inline blot it wraps the image in a
<span> and puts the class on the wrapper.
So the filter guarded a location the editor does not use, while span — which is
in ALLOWED_ATTRIBUTES with class — passed everything through. The protection
was in place, tested, and applied to the wrong element.
The lesson is not “remember to check spans”. It is:
A value-level allow-list must be keyed by the property being protected, not by the element you expect to carry it. Ask which elements can end up with this attribute? and answer it from the markup your editor actually emits, not from where the attribute logically belongs.
The practical version: dump the HTML your editor produces for each formatting feature, and read it. Every assumption in a sanitiser configuration is an assumption about someone else’s serialiser.
The empty shell a rejected iframe leaves
# Quand `src` est refusé, nh3 laisse la coquille `<iframe></iframe>` — que le
# navigateur dessine quand même en cadre vide de 300×150. On la retire.
# S'applique à du HTML DÉJÀ assaini, jamais à la saisie brute.
_EMPTY_IFRAME = re.compile(r"<iframe(?![^>]*\ssrc=)[^>]*>\s*</iframe>", re.I)
Returning None for a bad src removes the attribute; it does not remove the
element. <iframe> without src is valid HTML and the browser renders it at its
default 300×150 — a grey rectangle in the middle of an article.
Security-correct, visually broken. So the shell is stripped afterwards.
The comment on the last line is the important one: this regex runs on
already-sanitised HTML, never on raw input. Regular expressions are not HTML
parsers and must never be the thing standing between an attacker and a page. Here
the security decision was made by nh3; the regex is cosmetic cleanup on output
that is already safe. That ordering is what makes it acceptable, and it is worth
writing down every time, because the next reader will otherwise reasonably assume
this is a security control.
Quill 2 does not write <ul>
This section is not about security at all, and it is the one that changes what readers see.
# Quill 2 n'écrit PLUS de `<ul>`. Une liste à puces et une liste numérotée
# sortent toutes les deux en `<ol>`, et c'est un attribut sur la ligne —
# `data-list="bullet"` ou `data-list="ordered"` — qui les distingue. Les puces
# ne sont dessinées que par la feuille de style de l'éditeur, laquelle n'est
# évidemment pas chargée sur le site public.
#
# Sans cette normalisation : on saisit des puces, on relit des puces, et la page
# publie « 1. 2. 3. ».
Quill 2 emits <ol data-list="bullet"> for a bullet list. The bullets exist only
in the editor’s own stylesheet. On the public site, that is a numbered list.
The failure is invisible to the author: they type bullets, they proofread bullets
in the editor, and the published page shows 1. 2. 3.
And it compounds:
# Le nettoyage `nh3` achève le travail en retirant `data-list`, absent de la
# liste blanche — même l'information permettant de rattraper le coup serait
# perdue.
data-list is not in the allow-list, so the sanitiser removes it. After
sanitisation, the information needed to fix this no longer exists. Which fixes
the order of operations:
return _with_credits(_EMPTY_IFRAME.sub("", nh3.clean(
# La normalisation des listes passe AVANT le nettoyage : c'est le seul
# moment où `data-list` existe encore (`nh3` le retire).
_normalise_lists(str(value)),
tags=ALLOWED_TAGS,
attributes=ALLOWED_ATTRIBUTES,
attribute_filter=_attribute_filter,
link_rel="noopener noreferrer",
)))
Normalise, then sanitise, then repair the output. Each stage is where it is for a reason, and the reasons are opposite: normalisation needs data the sanitiser will destroy; the iframe cleanup needs output the sanitiser has already made safe.
The accessibility argument is made too, and it is the one that would justify the work on its own:
# C'est aussi ce qu'attendent un lecteur d'écran et un moteur de recherche : un
# lecteur d'écran annonce « liste numérotée à trois éléments », et il l'annonce
# à tort.
A screen reader announces “numbered list, three items” — confidently, and
wrongly. Visual sighted readers see the wrong markers; screen-reader users are
told the wrong structure. <ul> versus <ol> is semantics, not styling.
One detail in the rebuild is worth extracting:
# Les listes se SUCCÈDENT si le type change en cours de route :
# trois puces puis deux numéros font deux listes, et non une liste
# bâtarde dont la moitié des marqueurs seraient faux.
if tag != current_tag and current:
chunks.append((current_tag, current))
current = []
Quill keeps mixed list types in a single <ol>. Rebuilding into one element
would make half the markers wrong. Splitting into consecutive <ul> and <ol>
blocks is what the author saw and what the semantics require. When one
container holds two kinds of thing, the fix is two containers, not a compromise
container.
And the invisible marker span:
#: Marqueur que Quill glisse en tête de chaque ligne pour y dessiner la puce. Il
#: n'a aucun sens hors de l'éditeur, et `contenteditable` sur une page publique
#: est au mieux inutile, au pire une zone de saisie fantôme.
_QL_UI = re.compile(r'<span[^>]*\bclass="[^"]*\bql-ui\b[^"]*"[^>]*>\s*</span>', re.I)
Editor chrome leaking into published output. contenteditable on a public page
gives a visitor a text cursor in the middle of an article — harmless, confusing,
and the sort of thing that gets reported as “the site is broken”.
Writing after the sanitiser
Image credits are applied at render time, and the two reasons given are both about the library being the source of truth:
# La mention est posée AU RENDU, pas à l'insertion. Deux raisons :
# · corriger un crédit en bibliothèque met à jour toutes les pages qui
# utilisent l'image — c'est la promesse même d'une bibliothèque ;
# · le HTML stocké reste celui que Quill sait relire (un `<figure>` en base
# traverserait mal l'aller-retour dans l'éditeur).
Baking the credit into the stored HTML at insertion would freeze it: a photographer’s name corrected in the library would stay wrong on forty pages. And it would put markup in the database that the editor cannot round-trip.
But rendering after nh3 means writing into HTML that nothing will check again,
and the code says so:
# `escape` explicite : on écrit APRÈS nh3, donc plus rien ne nous
# protège. Un crédit saisi « <script>… » doit ressortir en texte.
return (f'<span class="{wrapper}">{tag}'
f'<span class="ag-figure__credit">© {escape(mention)}'
f'</span></span>')
credit_line is a database field typed by a person. Interpolated into a string
that is then mark_safe‘d, it is a stored XSS — through a field nobody thinks of
as rich text, on a page that has a sanitiser.
Any transformation that runs after sanitisation is unprotected, and every value it interpolates must be escaped by hand. The presence of a sanitiser in the pipeline is exactly what makes this easy to forget.
The wrapper element choice is argued too, and against the obvious answer:
# `<span>` et NON `<figure>`, malgré la sémantique plus juste de ce
# dernier : l'éditeur place toujours l'image dans un `<p>`, et un
# `<figure>` y est du contenu de bloc — le navigateur referme alors le
# paragraphe d'autorité et laisse deux paragraphes vides autour, donc
# des trous verticaux dans le texte.
<figure> is the semantically correct element and it cannot be used here: block
content inside a <p> causes the parser to close the paragraph, leaving empty
paragraphs and vertical gaps. A <span> with display: block renders identically
and stays valid.
That is the right trade and the right way to record it: the better answer named, the reason it does not work stated, so nobody “improves” it back.
Anchors are computed, so id is not allowed
# Les ancres sont posées APRÈS le nettoyage, par notre code : `id` n'est donc
# pas à ajouter aux attributs autorisés de nh3. Un identifiant venu de
# l'éditeur serait une valeur de saisie ; celui-ci est calculé.
A table of contents needs id on headings. The tempting move is to add id to
ALLOWED_ATTRIBUTES and let the editor supply it.
Instead the ids are generated from the heading text after cleaning. id stays
out of the allow-list entirely, so an id in stored content cannot survive — which
removes a class of problems that has nothing to do with XSS: an author-supplied id
colliding with id="header" or id="main" and breaking the page’s own
JavaScript.
The distinction — a value from the editor is input; this one is computed — is worth carrying to any sanitiser configuration. Do not permit an attribute because you need its effect. Ask whether you need the user’s value, or just the effect.
def _anchor(text, used):
"""Deux titres identiques dans un même article ne sont pas une faute — « Mise
en place » peut revenir sous deux parties. Sans le suffixe, les deux
entrées du sommaire mèneraient au même endroit.
"""
base = slugify(text)[:60] or "section"
candidate, n = base, 2
while candidate in used:
candidate = f"{base}-{n}"
n += 1
used.add(candidate)
return candidate
or "section" for a heading that slugifies to nothing — a heading that is only an
emoji, or only punctuation. slugify returns "", and <h2 id=""> is not a
usable anchor.
One cleaning path, two consumers
def _clean(value):
"""Le nettoyage, isolé : `richtext` et `outline` doivent voir le MÊME
HTML, sans quoi les ancres du sommaire ne tomberaient pas sur celles du
corps."""
@register.filter
def outline(value):
"""Recalculé depuis la même source et par le même chemin que `richtext` :
c'est ce qui garantit que chaque entrée du sommaire tombe sur une ancre qui
existe. Un sommaire construit à part finirait par pointer à côté au premier
changement de règle de nettoyage.
"""
The table of contents and the body are produced by two template filters, and both
route through _clean and _with_anchors.
The alternative is a TOC built by parsing headings separately — which works, and drifts. Add a tag to the allow-list, change the list normalisation, adjust the anchor slug length, and the TOC’s ids and the body’s ids diverge. Nothing errors. The links just stop scrolling anywhere, and nobody notices because nobody clicks their own table of contents.
Two derived views of the same content must be derived by the same code, even at the cost of doing the work twice. Correctness by construction beats a consistency you have to remember.
And a small editorial judgement at the end:
"""Vide s'il y a moins de deux titres : un sommaire d'une seule ligne n'aide
personne et occupe la colonne."""
What to take away
|safeon editor content turns one phished password into persistent XSS. The threat model is your own compromised account, not a stranger.- Allow-list tags, then attributes, then values. The third level is where “an iframe may exist” becomes “an iframe may point at YouTube”.
- Parse URLs and compare
hostnameagainst a set. Substring matching on a URL is always wrong. - Key value filters on the property, not on the element you expect. Find out what your editor really emits — a wrapper span is the classic miss.
- Order matters both ways: normalise before sanitising when the sanitiser destroys the evidence; clean up after it when the fix needs safe output.
- Everything interpolated after the sanitiser must be escaped by hand.
- Compute ids rather than allowing
id. Ask whether you need the user’s value or only the effect. - Derive the table of contents through the same code as the body. A separate path silently drifts.