Есть ли XSS уязвимости в самописном санитайзере?

Есть код на PHP.
Если коротко - он чистит пользовательский ввод перед сохранением в базу.

<?php $payload = '...';  $sanitizedText = strip_tags($payload, '<br><b>'); $sanitizedText = preg_replace('/(s)s+/m', '\1', $sanitizedText);  $sanitizedText = preg_replace("/<([a-z][a-z0-9]*)[^<|>]*?(/?)>/si",'<$1$2>', $sanitizedText); $sanitizedText = nl2br($sanitizedText);  echo $sanitizedText;

Try online: https://onlinephp.io/c/6a70a

Это код моего коллеги. Сам я Front-end разработчик.
Сначала тут не было третьей строки - она удаляет атрибуты. После того, как я показал коллеге пейлоад, который эксплуатирует недостаточную фильтрацию - он дописал регулярку, которая вырезает аттрибуты.
Но во-первых, он бекенд разработчик и у него нет опыта написания XSS зондов/эксплойтов. А во вторых, он сам говорит, что плохо знает регулярные выражения.

Итак, после некоторого анализа, я обнаружил, что регулярку с легкостью проходит следующий классический пейлоад:

<b a="><b onclick="alert(1)">click me</b>

Но strip_tags режет атрибуты, в которых есть <.

Потом я нашел ещё один интересный момент в strip_tags:

Input:
<b/some>bold</b>

Output:
bold</b>

Но вот что-то интересное из этого я вытянуть не смог пока что.

Подскажите, есть ли ещё тут что-то интересное для обхода? Спасибо!

Дополнительно:

Прогоните примеры отсюда:
https://cheatsheetseries.owasp.org/cheatsheets/XSS...

Ответы:

Если говорить с практической точки зрения, то лично я бы не стал ковыряться в этом говнокоде, а выкинул его целиком. И сделал нормально:

1. перед сохранением в базу текст вообще не трогал
2. (опционально - валидация, которая не трогает текст, а может только выдать ошибку, что он не соответствует требованиям)
3. перед выводом:
   
   • сделать ему htmlspecialchars()
   • и отрендерить в маркдаун, чтобы вместо этих пещерных <b> и <br> поддерживалось натуральное форматирование переводов строк, списков, выделения, и прочего.

• С практической точки да, я с вами согласен на 110%.

  Но этот legacy модуль используется везде. И в админке и на веб морде для клиентов и в API интеграциях. Переделывать его для поддержки Markdown не представляется возможным. Уже само присутствие такого кода в core production приложения свидельствует о том, что в компании всё на вчера и за небольшие деньги. Никто не будет тратить ресурсы на обеспечение какой-то там секьюрити или тем более best practices.

  К сожалению, пока база клиентов не поедет на теневые форумы, никто пальцем об палец не ударит.

  К тому-же, у нас куча мест, где поддерживается заголовок `Accept: text/html`, а он просто рендерит шаблоны из данных из базы. Я не знаю, есть ли там вообще хоть какая-то санитизация.

  В общем, вопрос скорее не о том, "как нужно", а об эксплойте. И, скорее не для улучшения ситуации (зачем мне это нужно, если никому не нужно? Серьезность я объяснял всем по 100 раз. Они сделали выбор), а из азарта.

• К сожалению, пока база клиентов не поедет на теневые форумы, никто пальцем об палец не ударит.

  Так давай же ускорим этот момент, раз вам с другом бекендером так скучно, выкладывай урл))

• Uno, хорошая попытка, молодой человек (с)

Если речь про очистку html то могу посоветовать только 2 проверенных решения
1. https://github.com/ezyang/htmlpurifier
2. https://symfony.com/doc/current/html_sanitizer.html
Все остальные велосипеды - до первого взлома.

• Нет, речь про взлом конкретного санитайзера