За последние 24 часа нас посетили 26464 программиста и 1545 роботов. Сейчас ищут 822 программиста ...

Правильно ли я написал защиту от csrf-атак?

Тема в разделе "PHP для новичков", создана пользователем Walk, 28 фев 2018.

  1. Walk

    Walk Активный пользователь

    С нами с:
    7 сен 2008
    Сообщения:
    452
    Симпатии:
    86
    Для самописного фреймворка продумываю защиту от csrf-атак.
    Подскажите, правильно ли она сделана в этом примере:

    PHP:
    1. <?php
    2.  
    3. function getRandomString($length = 15) {
    4.     $chars = '1234567890ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz';
    5.     $numChars = strlen($chars);
    6.     $string = '';
    7.     for ($i = 0; $i < $length; $i++) {
    8.         $string .= substr($chars, rand(1, $numChars) - 1, 1);
    9.     }
    10.     return $string;
    11. }
    12.  
    13.  
    14. if ($_POST) {
    15.     echo '<pre>';
    16.     print_r($_POST);
    17.     echo '</pre>';
    18.  
    19.     if ($_POST['csrf'] === $_SESSION['csrf']) {
    20.         echo '<p>Токены идентичны. Ваши данные приняты.</p>';
    21.     } else {
    22.         echo '<p>Токены не совпадают. Ваши данные отклонены.</p>';
    23.     }
    24. } else {
    25.     $_SESSION['csrf'] = getRandomString();
    26. }
    27. ?>
    28.  
    29. <form method="POST" action="">
    30.     <input type="hidden" name="csrf" value="<?= $_SESSION['csrf'] ?>">
    31.  
    32.     <input type='text' maxlength='15' value='' name='Name'  />
    33.  
    34.     <input type="submit">
    35. </form>
    Результат отправки данных:

    [​IMG]
     
    AlexandrS нравится это.
  2. mkramer

    mkramer Суперстар
    Команда форума Модератор

    С нами с:
    20 июн 2012
    Сообщения:
    8.598
    Симпатии:
    1.764
  3. Fell-x27

    Fell-x27 Суперстар
    Команда форума Модератор

    С нами с:
    25 июл 2013
    Сообщения:
    12.156
    Симпатии:
    1.771
    Адрес:
    :сердА
    Эта штука, чисто в теории, может сгенерить строку, которая будет некорректно парситься в HTML.
     
  4. [vs]

    [vs] Суперстар
    Команда форума Модератор

    С нами с:
    27 сен 2007
    Сообщения:
    10.559
    Симпатии:
    632
    Неправильная реализация, токен не одноразовый
    --- Добавлено ---
    И если открыть две вкладки, работать будет только одна
     
  5. Fell-x27

    Fell-x27 Суперстар
    Команда форума Модератор

    С нами с:
    25 июл 2013
    Сообщения:
    12.156
    Симпатии:
    1.771
    Адрес:
    :сердА
    да не должен он быть одноразовым. Достаточно ему быть односессионным. Одноразовость - излишнее усложнение, не дающее никакого профита. Другое дело, что сессии не должны быть "вечными".
     
    romach нравится это.
  6. [vs]

    [vs] Суперстар
    Команда форума Модератор

    С нами с:
    27 сен 2007
    Сообщения:
    10.559
    Симпатии:
    632
    @Fell-x27 судя по коду, ТС. пытался сделать его одноразовым, но получилось, что токен затирается при рефреше, но сохраняется при выполнении действия. Про одноразовость. Допустим, пользователь отправил кому-то ссылку из адресной строки, содержащую токен. Если он одноразовый, то это означает, что он уже отыграл при рендере страницы и теперь бесполезен.
     
  7. keren

    keren Новичок

    С нами с:
    15 ноя 2017
    Сообщения:
    513
    Симпатии:
    42
    Не должно быть рефреша при пост запросе, там защита пост данных.
     
  8. [vs]

    [vs] Суперстар
    Команда форума Модератор

    С нами с:
    27 сен 2007
    Сообщения:
    10.559
    Симпатии:
    632
    так при посте, токен не затирается в этом коде
     
  9. keren

    keren Новичок

    С нами с:
    15 ноя 2017
    Сообщения:
    513
    Симпатии:
    42
    и не должен, токен берется из сессии в скрытое поле и параллено отправляется с основными данными, если это то что хотел тс.
     
  10. [vs]

    [vs] Суперстар
    Команда форума Модератор

    С нами с:
    27 сен 2007
    Сообщения:
    10.559
    Симпатии:
    632
    да, но при простом рефреше токен затирается новым - в чем логика?
     
  11. keren

    keren Новичок

    С нами с:
    15 ноя 2017
    Сообщения:
    513
    Симпатии:
    42
    не знаю что за простой рефреш, но пост данные не должны рефрешится и должен быть хедер с редиректом автоматом.
     
  12. artoodetoo

    artoodetoo Суперстар
    Команда форума Модератор

    С нами с:
    11 июн 2010
    Сообщения:
    11.128
    Симпатии:
    1.248
    Адрес:
    там-сям
    Я бы всё-таки привязал токен к конкретному экшену/форме, чтобы разные формы одного пользователя не конкурировали )))
    То есть не $_SESSION['csrf'], а $_SESSION['csrf']['action_X']
     
  13. Walk

    Walk Активный пользователь

    С нами с:
    7 сен 2008
    Сообщения:
    452
    Симпатии:
    86
    Да, есть такая недоработка

    Сделал такой вариант, можно открывать несколько страниц с формой - везде будет корректная проверка. Но не знаю, на сколько этот вариант "кошерный" - при каждом обновлении страницы с формой создается новая запись с идентификатором и хешем формы:

    PHP:
    1. <?php
    2.  
    3. function getRandomString($length = 15) {
    4.     $chars = '1234567890ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz';
    5.     $numChars = strlen($chars);
    6.     $string = '';
    7.     for ($i = 0; $i < $length; $i++) {
    8.         $string .= substr($chars, rand(1, $numChars) - 1, 1);
    9.     }
    10.     return $string;
    11. }
    12.  
    13.  
    14. if ($_POST) {
    15.  
    16.     $token = explode('_', $_POST['csrf']);
    17.  
    18.     if (hash_equals($token[0].'_'.$token[1].'_'.$token[2], $_SESSION['csrf'].'_'.$token[1].'_'.$_SESSION['form'][$token[1]])) {
    19.         echo '<p>Токены идентичны. Ваши данные приняты.</p>';
    20.     } else {
    21.         echo '<p>Токены не совпадают. Ваши данные отклонены.</p>';
    22.     }
    23. } else {
    24.  
    25.     // Создаем ключ сесси, если его нет. Не перезаписывается
    26.     if (!isset($_SESSION['csrf'])) {
    27.         $_SESSION['csrf'] = getRandomString();
    28.     }
    29.  
    30.     // Создаем массив с хешами форм - при каждом обновлении страницы с формой создается новая запись со своим хешем
    31.     for ($i = 0;; $i++) {
    32.         if (!isset($_SESSION['form'][$i])) {
    33.             $_SESSION['form'][$i] = getRandomString();
    34.             break;
    35.         }
    36.     }
    37. }
    38.  
    39. ?>
    40.  
    41. <form method="POST" action="">
    42.     <input type="hidden" name="csrf" value="<?= $_SESSION['csrf'].'_'.$i.'_'.$_SESSION['form'][$i] ?>">
    43.  
    44.     <label for="name">Имя:</label>
    45.     <input id="name" maxlength='15' value='' name='Name'  />
    46.  
    47.     <input type="submit">
    48. </form>
     
  14. Fell-x27

    Fell-x27 Суперстар
    Команда форума Модератор

    С нами с:
    25 июл 2013
    Сообщения:
    12.156
    Симпатии:
    1.771
    Адрес:
    :сердА
    А смысл? Кроме, опять же, излишнего усложнения. О какой конкуренции речь?
     
  15. mkramer

    mkramer Суперстар
    Команда форума Модератор

    С нами с:
    20 июн 2012
    Сообщения:
    8.598
    Симпатии:
    1.764
    Ну я столкунлся с такой проблемой на ранних релизах Yii2 с его CSRF-защитой (сейчас этого вроде не наблюдается). Чувак открыл в сделанной мной CMS статью в админке на редактирование, а в соседней вкладке - сайт, чтоб взять ссылки на перелинковку, и в результате статья не добавилась - открывшийся сайт перезаписал CSRF-токен в сессии. Сейчас как-то по-хитрому решено это в Yii2, я недавно трассировал его, но там так сразу и не въехать, хотя по строчкам решение не много занимает.
     
  16. Fell-x27

    Fell-x27 Суперстар
    Команда форума Модератор

    С нами с:
    25 июл 2013
    Сообщения:
    12.156
    Симпатии:
    1.771
    Адрес:
    :сердА
    Вот я и говорю, гумно эти ваши одноразовые хитровыбоенные формо-зависимые уникальные токены, и кроме геморроя и дутых проблем ничего не несут. Профиту с них, что коту с электробритвы.

    Люди упарываются, боясь, что в реальном времени кто-то тырить будет эти токены, забывая, что если кто-то имеет доступ к клиентской машине на таком уровне, то ему проще будет стырить явки-пароли, или напрямую действовать от имени клиента втихоря, а не тырить csrf-токены. csrf-токены это про рандомные нетаргетированные атаки через хитрые ссылочки.
     
  17. artoodetoo

    artoodetoo Суперстар
    Команда форума Модератор

    С нами с:
    11 июн 2010
    Сообщения:
    11.128
    Симпатии:
    1.248
    Адрес:
    там-сям
    код из первого поста будет создавать проблемы если открыто несколько форм. вот о какой.

    Ну буду ничего утверждать также безапеляционно :) Просто замечу, что используются разные техники.

    Полезное: https://habrahabr.ru/post/318748/

     
  18. Walk

    Walk Активный пользователь

    С нами с:
    7 сен 2008
    Сообщения:
    452
    Симпатии:
    86
    Второй вариант вам нравится? Там такой проблемы нет.

    Хотя мой внутренний перфекционизм не доволен ни первым, ни вторым вариантом - смотрю сейчас разные микро-фреймворки - как в них реализована защита от csrf-атак.

    В некоторых её вообще нет)
     
  19. Fell-x27

    Fell-x27 Суперстар
    Команда форума Модератор

    С нами с:
    25 июл 2013
    Сообщения:
    12.156
    Симпатии:
    1.771
    Адрес:
    :сердА
    Ну тогда надо исправлять корень проблемы, а не лепить костыли на следствия :)
    --- Добавлено ---
    Я бы не был столь уверен в полезности этого материала, если честно. Там рассказывается, как с помощью адронного коллайдера гвоздь в фанерку забить.
     
  20. artoodetoo

    artoodetoo Суперстар
    Команда форума Модератор

    С нами с:
    11 июн 2010
    Сообщения:
    11.128
    Симпатии:
    1.248
    Адрес:
    там-сям
    ой всё! :) Фел, ты молодец. ок? и идём дальше.

    самоочевидно, что дырка в защите в одном месте обнуляет всю защиту целиком. если есть возможность присунуть исполняемый JS куда-нибудь, то аккаунт админа будет взломан с вероятностью 103%. НО, сцуко, всегда есть "но". в каких-то случаях токен может быть просран и без JS-инъекции, просто по невнимательности или в силу обстоятельств, которые изначально не были рассмотрены. цитирую https://ru.wikipedia.org/wiki/Межсайтовая_подделка_запроса :

    «…но следует иметь в виду, что спецификация HTTP/1.1 [3] допускает наличие тела для любых запросов, но для некоторых методов запроса (GET, HEAD, DELETE) семантика тела запроса не определена, и должна быть проигнорирована. Поэтому ключ может быть передан только в самом URL, или в HTTP заголовке запроса. Необходимо защитить самого пользователя от неблагоразумного распространения ключа, в составе URL…»

    я здесь вижу увеличение вероятности случайного просера токена. и если он один универсальный на любые действия пользователя в пределах сессии, это уже выглядит опасно.
    --- Добавлено ---
    или, например, операция logout. есть старая добрая шутка, когда подсовывают ссылку на "картинку" с адресом который всех посетителей разлогинит. чтобы защититься, надо подмешать токен. здесь ссылка выглядит так:
    https://php.ru/forum/logout/?_xfToken=23761/1519923172-6fa53b729f9ee86d88b8a04d9ea0b72935463560

    (ойбля, я только что просрал свой токен. а ведь я модератор. хакеры сейчас воспользуются)
    --- Добавлено ---
    так что лучше без категоричных заявлений. всегда есть "но" и особые обстоятельства и хрена ты их все заранее учтёшь.
     
  21. Walk

    Walk Активный пользователь

    С нами с:
    7 сен 2008
    Сообщения:
    452
    Симпатии:
    86
    ну, на нормальных проектах вход в админку находится на другом домене - токен с основного домена тут никак не поможет.

    плюс к этому, самое важное - хеш от пароля хранится в куках

    P.S. Вы никак не ответили на мой вопрос:
     
  22. Fell-x27

    Fell-x27 Суперстар
    Команда форума Модератор

    С нами с:
    25 июл 2013
    Сообщения:
    12.156
    Симпатии:
    1.771
    Адрес:
    :сердА
    Ок, я модератор, я могу выпилить к чертовой матери весь форум мановением мышки. Но вход в мой аккаунт не на отдельном домене.
    --- Добавлено ---
    это еще одна вещь, которую делать нельзя.
     
  23. romach

    romach Старожил

    С нами с:
    26 окт 2013
    Сообщения:
    2.904
    Симпатии:
    719
    А зачем его передавать GET-запросом?
    --- Добавлено ---
    Из той же статьи в вики:
    --- Добавлено ---
    Ну, я категорично заявляю, что в данном случае @Fell-x27 прав, потому как нет ни одного разумного довода в пользу обратного )
     
  24. artoodetoo

    artoodetoo Суперстар
    Команда форума Модератор

    С нами с:
    11 июн 2010
    Сообщения:
    11.128
    Симпатии:
    1.248
    Адрес:
    там-сям
    зачем его передавать DELETE запросом понятно? он там тоже упоминается

    "нет сведений" == "я не видел" / "я игнорирую сведения". потому что довольно много людей считают иначе.
    --- Добавлено ---
    друзья, если кратко, лично мне насрать считаете ли вы достаточным тот или иной метод защиты :) или считаете ли вы достаточным, когда кто-то приводит иные аргументы.

    ваша защита это ваша проблема. с моей т.з. защита может быть только откровенно плохой или вроде бы хорошей. и не бывает гарантированно достаточной.

    --- Добавлено ---
    а должен был? :) при беглом взгляде мне не понравилось. как-то криво там со счётчиком $i, а вникать и исправлять лень.
     
  25. romach

    romach Старожил

    С нами с:
    26 окт 2013
    Сообщения:
    2.904
    Симпатии:
    719
    нет.