Skip to content

Sécurité des liens : un seul validateur d'URL et rel="noopener" avec target="_blank" - #52

Open
jmcollin wants to merge 2 commits into
mainfrom
refactor/shared-url-validator
Open

jmcollin wants to merge 2 commits into
mainfrom
refactor/shared-url-validator

Conversation

@jmcollin

@jmcollin jmcollin commented Oct 8, 2026

Copy link
Copy Markdown
Owner

Empilée sur #50, le correctif urgent de main. Merger #50 d'abord.

Problème (points 13 et 14 de la review)

13. Deux validations d'URL qui divergeaient

InlineParser::isSafeLinkUrl() HtmlSanitizer::isSafeUrl()
Espaces en début ou fin retirés avant le contrôle URL rejetée
Espaces internes (/my uri) acceptés URL rejetée
%6Aavascript: accepté (inoffensif) rejeté

Deux règles différentes pour la même question : c'est le genre d'écart qui a produit le bypass corrigé dans #46.

14. Avec seulement linkTarget: '_blank', le renderer émettait target="_blank" sans rel="noopener" (risque de reverse tabnabbing), alors que le sanitizer l'impose déjà pour le HTML brut.

Correctif

  • Nouveau Sanitizer\UrlValidator::isSafe(), utilisé par l'InlineParser (liens, images, références) et par le HtmlSanitizer (href et src). Il garde la règle la plus stricte de chaque version :
    • caractères de contrôle rejetés, bruts ou percent-encodés ;
    • espaces de début et de fin retirés avant le contrôle du schéma, comme le font les navigateurs ;
    • URL protocol-relative (//…) et préfixées par \ rejetées ;
    • allowlist de schémas (http, https, mailto, relatif) appliquée à l'URL et à sa forme percent-décodée.
  • La liste des schémas exécutables des autoliens (javascript, vbscript, data) y est déplacée : UrlValidator::hasScriptScheme().
  • Renderer : target="_blank" ajoute toujours noopener, ou le complète si linkRel est fourni, sans doublon et sans tenir compte de la casse. Les autres valeurs de target et les autoliens e-mail ne changent pas.

Changements de comportement mineurs

  • Le sanitizer accepte maintenant les URL avec espaces internes (<a href="a%20b">), comme les liens Markdown.
  • Un lien Markdown relatif sans / qui contient %3A (par exemple foo%3Abar) est rejeté, puisque sa forme décodée ressemble à un schéma.

Tests

  • Nouveau tests/Unit/Sanitizer/UrlValidatorTest.php : 9 URL sûres, 14 vecteurs à rejeter, détection des schémas exécutables.
  • Nouveau tests/Unit/Renderer/LinkTargetRelTest.php : 4 tests sur noopener. Il n'existait aucun test sur linkTarget/linkRel.
  • Toute la suite de sécurité existante passe sans modification.
  • PHPUnit OK (945 tests), PHPStan et Psalm sans erreur.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CPcuaaxrCottkeL1ykoSWe


Generated by Claude Code

claude added 2 commits October 8, 2026 10:05
#44 (blockquote laziness) and #45 (table detection) were merged in that
order. #44 used PATTERN_TABLE_SEPARATOR in quoteLineState(), #45 deleted
the constant, so every blockquote now throws
"Error: Undefined constant Lexer::PATTERN_TABLE_SEPARATOR" on main
(40 test errors).

The check added nothing to blockquote laziness; drop it.

(cherry picked from commit 7e799b1, pushed to the #44 branch after #44
had already been merged)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CPcuaaxrCottkeL1ykoSWe
…links

Two URL safety checks had drifted apart: InlineParser::isSafeLinkUrl()
(trims spaces, allows inner spaces) and HtmlSanitizer::isSafeUrl()
(decodes then rejects any space, no trimming). Both now call
Sanitizer\UrlValidator::isSafe(), which keeps the stricter rule of each:
controls rejected raw or percent-encoded, leading/trailing spaces
trimmed before the scheme check, protocol-relative and backslash URLs
rejected, and the scheme allowlist applied to both the URL and its
percent-decoded form. The autolink script-scheme list moves there too.

The renderer emitted target="_blank" without rel="noopener" when only
linkTarget was configured (reverse tabnabbing on older browsers), while
HtmlSanitizer already enforced it for raw HTML links. noopener is now
added (or appended to linkRel) for _blank targets.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CPcuaaxrCottkeL1ykoSWe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants