https://github.com/Cotonti/Cotonti/issues/1431

Добавлено 6 дней спустя:

Для информации:

как показывает проведенное на локальном сайте исследование (за наводку на Acunetix — автору топика отдельное спасибо) проблемных мест несколько больше, чем может показаться на первый взгляд.

Работа над фиксами идет. 

Добавлено 3 дня спустя:

	$out['canonical_uri'] = empty($out['canonical_uri']) ? str_replace('&', '&', $sys['canonical_url']) : $out['canonical_uri'];
	$out['canonical_uri'] = strip_tags($out['canonical_uri']);
	$out['canonical_uri'] = htmlentities($out['canonical_uri'], ENT_QUOTES, "UTF-8");
	$out['canonical_uri'] = htmlspecialchars($out['canonical_uri'], ENT_QUOTES);

 

после этого ругаться перестал, вопрос к знающим людям, подскажите где тут ошибки

На вскидку по приведенному куску кода:
1. `htmlentities`, судя по документации, покрывает `htmlspecialchars` поэтому он тут лишний.
2. `htmlentities` в том виде, как он используется здесь отработает не правильно, т.к. повторно экранирует например & и получится &
Надо четвертый параметр в `false` поставить.
3. По идее, при экранировании через `htmlentities` удаление тегов будет излишне, т.к. < > уже будут заменены.
4. Т.к. мы фильтруем не просто html а URI, то применять тут `htmlentities` нельзя. `htmlentities` выдает «entity» которые разрушат формат строки параметров, т.к. в них содержится символ «&».

Главный момент — фильтровать данные тут и таким образом не правильно. По двум причинам:
1. Здесь в `$out['canonical_uri']` у нас лежит уже готовый, сформированный URI. А XSS код приходит к нам в части path или query. Если говорить о формате данных — его определяет RFC3986 и все недопустимые символы должны быть кодированы в формат %xx (см.п. 2.1).
2. Далее... фильтровать данные, по хорошему, надо на «входе», в не на «выходе». Иначе имеем шанс, что их успеет использовать еще какой-либо плагин. 
Корень проблемы в `$sys['canonical_url']`, в который пишется не фильтрованная строка `$_SERVER['REQUEST_URI']`. Для фильтрации URL применяется ф-я `rawurlencode()`, но здесь она в чистом виде нам не поможет, т.к. а). часть данных уже может быть закодирована; б). нам не надо кодировать некоторые специальные символы (/?&=[]). 

Т.е. алгоритм примерно такой: 
1. разбить `$_SERVER['REQUEST_URI']` на path и query
2. разбить path по `/`, 
3. проверсти кодирование каждого элемента (сначала раскодировать, потом повотрно закодировать)
4. раскодировать query, преобразовать в массив (cot_parse_str)
5. собрать обратно массив параметров через http_build_query (тут есть нюансы — см. как реализовано внутри cot_url)
6. склеить обратно path и query, и вернуть в `$_SERVER['REQUEST_URI']`

Ну вот как-то так.

Добавлено 3 недели спустя:

Правки внесены. Тикет №1431 закрыт.

Кроме разных фиксов в `functions.php`  добавлена функция `cot_url_sanitize()`, которая используется для фильтрации(кодирования) недопустимых символов в canonical URL.

$cfg['display_errors'] = FALSE;        // Display error messages. Switch it FALSE on production sites

ща попробую на виртуалке

Скачал себе и решил сам посканить, посмотреть что скажет.

В целом вроде хорошо, но кое-что - мелочи нашлись.

rc.php - у меня включено сжатие css\js

и он на него массив начал слать - в итоге прямым текстом выдало - php ошибка в такой-то строке в файле /var/www/итд

сделал проверку строки - поменял

if (isset($_GET['rc']) && preg_match('#^[\w\.\-]+\.(js|css)$#', $_GET['rc'], $mt))
{
	$src_uri = $cfg['cache_dir'] . '/static/' . $_GET['rc'];
	$content_type = $mt[1] == 'js' ? 'text/javascript' : 'text/css';
}

на

if (isset($_GET['rc']) && is_string($_GET['rc']))
{
	if (preg_match('#^[\w\.\-]+\.(js|css)$#', $_GET['rc'], $mt)){
	$src_uri = $cfg['cache_dir'] . '/static/' . $_GET['rc'];
	$content_type = $mt[1] == 'js' ? 'text/javascript' : 'text/css';
	}
}

далее - system/function.php

при проверке других файлов, кажется он докопался к login и начал в него кидать массив, выдал ошибки function, в которых был путь к файлу на сервере

в 2 местах добавил проверку

~432 строка

	if ($maxlen>0)
	{
		/* string test*/
		if (is_string($v)) {
		$v = mb_substr($v, 0, $maxlen);
		}
	}

и нем же еще 1 проверка на строку

~471

		case 'TXT':
		/* string test*/
			if (is_string($v))
			{
				$v = trim($v);
				if (mb_strpos($v, '<')===FALSE)
				{
					$pass = TRUE;
				}
			else
			{
				$defret = str_replace('<', '&lt;', $v);
			}
			}
			break;

и последнее на что оно сильно ругалось - xss в каноникале - что можно скрипт использовать

поменял в header.php

	$out['canonical_uri'] = empty($out['canonical_uri']) ? str_replace('&', '&', $sys['canonical_url']) : $out['canonical_uri'];
	$out['canonical_uri'] = strip_tags($out['canonical_uri']);
	$out['canonical_uri'] = htmlentities($out['canonical_uri'], ENT_QUOTES, "UTF-8");
	$out['canonical_uri'] = htmlspecialchars($out['canonical_uri'], ENT_QUOTES);

после этого ругаться перестал, вопрос к знающим людям, подскажите где тут ошибки

тк я не программист, а только учусь)