Hannah Lin
Where you sanitise matters more than what you sanitise with. Render once on write, store the result, and keep the sanitiser off the hot path.
Link copied to clipboard
A markdown blog has one genuine security problem: the author's markdown becomes HTML, and HTML can carry script. Everything else is a detail about where to do the conversion.
The two bad options
Sanitise on read
Every request parses the markdown, renders it, and sanitises the result. This is the default in most setups, and it has three problems:
- It burns CPU regenerating identical output for content that changed three months ago.
- The sanitiser is on the request path, so a bypass is an active vulnerability rather than a stored-data problem you can find and fix.
- It makes the render function a hot dependency that must be correct for every request, forever.
Sanitise on the client
Ship a markdown parser and a sanitiser, render in the browser. Now you have three problems: the client sanitiser has a different bypass surface than the server one, a client-side bypass is exploitable by anyone reading the page, and you have added ~90KB to every page load to render content that never changes.
The option that is actually good
Render and sanitise once, at write time, store the sanitised HTML alongside the markdown, and never parse on read.
write: markdown ──► parse ──► sanitise ──► body_html (stored)
read: body_html ──► {!! !!} (no parsing, no sanitising)Three consequences, in decreasing order of importance:
- The sanitiser runs once per edit, not once per view. A bypass affects stored content, which is a data-integrity problem with a clear fix: purge and re-render. It is not a live vulnerability.
- Reads are a single column fetch. The public page does no parsing at all.
- There is exactly one place to audit. A grep for
body_htmlreturns every write site.
The critical constraint
The sanitiser must be the only writer of the rendered column. The moment a second code path can write it — an admin importer, a console command, a seeder that hand-builds HTML — you have reintroduced the problem the design was meant to eliminate.
A model that declares body_html non-fillable is what enforces this:
#[Fillable([
'title', 'slug', 'excerpt', 'body',
'status', 'is_featured', 'published_at', 'category_id',
])]
class Post extends Model
{
// body_html, body_text, word_count, reading_time are absent
// by design: they are derived, and only the rendering
// pipeline may write them.
}Because they are not fillable, Post::create($request->all()) cannot inject a pre-rendered body_html. Because they are written via forceFill() inside one action, a grep finds every writer.
What the sanitiser must actually do
Four things, and a surprising number of implementations miss at least one:
Allow-list tags, do not deny-list them. A deny list of <script> and <iframe> is a permanent game of catch-up against <svg onload>, <math href>, and the rest. Name what is permitted and drop everything else.
protected const array ALLOWED_TAGS = [
'p', 'br', 'hr', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6',
'strong', 'em', 'del', 'code', 'pre', 'kbd', 'mark',
'ul', 'ol', 'li', 'blockquote', 'a',
'table', 'thead', 'tbody', 'tr', 'th', 'td', 'img',
];Allow-list attributes per tag. <a> gets href, title, target, rel. It does not get onclick, style, or formaction.
Reject dangerous URL schemes. javascript: and data: are the two that matter; parse_url() plus a scheme allow-list closes both:
protected function isUnsafeUrl(string $url): bool
{
$scheme = strtolower((string) parse_url(trim($url), PHP_URL_SCHEME));if ($scheme === '') {
return false; // Relative URL: safe.
}
return ! in_array($scheme, ['http', 'https', 'mailto'], true);
}
```
Escape before you mark up, not after. The ordering inside the inline renderer is the whole ballgame: escape the author's text first, then apply **bold** and [link](url) transformations. Reversing that order lets **<script>** through as markup.
The test that proves it
test('raw script tags in markdown never reach rendered html', function () {
$html = app(MarkdownRenderer::class)->render(
"<script>alert('xss')</script>\n\nNormal **text**."
);expect($html)
->not->toContain('<script')
->not->toContain('alert');
});
test('javascript urls are stripped from links', function () {
$html = app(MarkdownRenderer::class)
->render('click)');
expect($html)->not->toContain('javascript:');
});
```
Both assert on the rendered output rather than on the sanitiser's internals, so they keep working if the implementation changes and keep failing if the property is lost.
The generalisable principle
For any transformation between untrusted input and rendered output, ask two questions: where does it run, and how many things can do it.
Running it on write bounds the blast radius to stored data. Having exactly one writer makes the audit tractable. Getting those two properties right matters far more than which sanitiser library you picked, because a correct library wired to two write paths is still an open door.
Keep reading
Discussion
-
HWrote something very similar last year but your framing of the trade-off is much clearer. Thanks for publishing.
-
A
Anika Sorensen
Bookmarking for our next sprint retro. The code review section is going straight into the team handbook.