Страницы

Поиск по вопросам

среда, 10 июля 2019 г.

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

в Laravel есть класс VerifyCsrfToken, в котором определена функция handle, которая ловит request и проверяет соответствие токена клиента и сервера, после добавляет в response куку, иначе throw error.
public function handle($request, Closure $next) { if ( $this->isReading($request) || //проверяет метод формы на соотв. из массива ['HEAD','GET',...] $this->runningUnitTests() || $this->shouldPassThrough($request) || $this->tokensMatch($request) ){ return $this->addCookieToResponse($request, $next($request)); }
throw new TokenMismatchException; }
Смотрим на условие и видим, что проверяется 4 функции, приоритет при || начинается слева, то есть (здесь скорее всего, я ошибаюсь) у tokenMatch самый маленький приоритет и в случае, если первые 3 функции вернут true, тогда эта функция не будет учитываться в условии, что плохо.
И собственно вопрос, для гуру-laravel, мои рассуждения верны?


Ответ

Главное - да, ваши рассуждения верны. Если любой из первых 3 методов вернёт true, то tokensMatch даже не будет вызываться. Это применимо к любому PHP-коду.
Теперь посмотрим, является ли это ошибкой или так запланировано. Если запланировано, то зачем и не является ли это угрозой.
$this->isReading($request) || //проверяет метод формы на соотв. из массива ['HEAD','GET',...]
Проверяет, является ли текущий запрос читающим. Напомню, что согласно спецификации HTTP запросы типов GET, HEAD не должны изменять состояние системы. А раз они не меняют состояние системы - то их банально и не нужно защищать от CSRF атак.
$this->runningUnitTests() ||
На самом деле костыль, обычно код не должен различать, запущен он в окружении юнит-тестов или нет. Видимо, так было проще. Во всяком случае суть в том, что для запуска из юнит-тестов проверка всегда проходит.
$this->shouldPassThrough($request) ||
Здесь без чтения исходника мне сложно сказать, что проверяется. Исходник Ага, проверяется список исключений для URI, для которых проверку CSRF надо пропустить. По-умолчанию список пуст, но возможность отстрелить себе ногу есть.
$this->tokensMatch($request)
Наконец, если запрос что-то хочет изменить, не в окружении юнит-тестов и не в списке исключений, то проверяем валидность токена.
На мой взгляд всё логично, ошибки в безопасности здесь нет. tokensMatch действительно должен вызываться в самом конце.
Потенциальная проблема с isReading. Многие люди не учитывают семантику GET запросов и делают, например, удаление сущности ссылкой из-за удобства написания простой ссылки, а не целой формы. Не надо так делать.

Комментариев нет:

Отправить комментарий