fixed rtrim(DB()->escape($info_hash), ' ')

sхс

Legend
TP version
other
Вот это очень злая штука в announce.php. Зачем нам экранировать хэш? Нам надо сохранить строку в таком виде какая она есть. И что это за история с обрезанием пробела в конце инфо хэша?

Я бы не придирался к этому, но однажды я не смог зарегистрировать торрент, по причине того, что такой уже существует на трекере, хотя раздаваемые файлы были абсолютно разные и это случилось из-за DB()->escape() и наличии последоветельности \' в info_hash еще до экранирования

Если я заблуждаюсь по этому поводу, дайте знать

пока в announce.php стоит так
PHP:
    $info_hash_sql = bin2hex($info_hash);
 
    $info_hash_sql2 = bin2hex(rtrim($info_hash, ' ')); // старые торренты отвалилисиь пришлось вернуть этот костыль
    $info_hash_sql3 = bin2hex(rtrim(DB()->escape($info_hash), ' '));// старые торренты отвалилисиь пришлось вернуть этот костыль
 
    $passkey_sql   = DB()->escape($passkey);

    // OR HEX(tor.info_hash2) LIKE '$info_hash_sql%' - непонятный экспромт qbittorent разрабов, режут 32битный хэш до 20bit для совместимости
    $sql = "
        SELECT tor.topic_id, tor.poster_id, tor.tor_type, u.*, tor.complete_count
        FROM ". BB_BT_TORRENTS ." tor
        LEFT JOIN ". BB_BT_USERS ." u ON u.auth_key = '$passkey_sql'
        LEFT JOIN bb_users u1 ON u1.user_id = u.user_id
        WHERE HEX(tor.info_hash) = '$info_hash_sql'
            OR HEX(tor.info_hash) = '$info_hash_sql2'
            OR HEX(tor.info_hash) = '$info_hash_sql3'
            OR HEX(tor.info_hash2) = '$info_hash_sql'
            OR HEX(tor.info_hash2) LIKE '$info_hash_sql%'
        LIMIT 1
    ";

в function tracker_register(
PHP:
    $info_hash     = pack('H*', sha1(bencode($info)));
    $info_hash2    = pack('H*', hash('sha256', bencode($info)));
    //$info_hash_sql = rtrim(DB()->escape($info_hash), ' ');
    $info_hash_sql = bin2hex($info_hash);
    $info_hash2_sql= bin2hex($info_hash2);
    $info_hash_md5 = md5($info_hash);

    $sql = "SELECT topic_id
        FROM ". BB_BT_TORRENTS ."
        WHERE HEX(info_hash) = '$info_hash_sql' OR HEX(info_hash2) = '$info_hash2_sql'
        LIMIT 1";

        .....
    $columns = ' info_hash, info_hash2,    post_id,  poster_id,  topic_id,  forum_id,  attach_id,    size,  reg_time,  tor_status';
    $values = "x'$info_hash_sql', x'$info_hash2_sql', $post_id, $poster_id, $topic_id, $forum_id, $attach_id, '$size', $reg_time, $tor_status";
 
Last edited:
rtrim() стоял в целях совместимости, можете избавиться.
escape() же должен присутствовать, иначе sql инъекция, проблем связанных с ним я не встречал.

Да, бинарная строка экранируется, но это не мешает поиску.
Но если у вас наблюдается что-то, то можно поступить без экранирования:

PHP:
// $info_hash_sql3 = bin2hex(rtrim(DB()->escape($info_hash), ' '));
if (!ctype_xdigit(bin2hex($info_hash))) {
    msg_die("Invalid info hash");
}
 
Так а если у меня info_hash тип varbinary(20) и я туда отправляю bin2hex($info_hash) это тоже грозит инъекцией?
 
ctype_xdigit() я предложил только для проверки в анонсере, если хотите избавиться от этапа экранирования строки (работоспособность в запросе я не проверял).

Я не могу никак понять вашу цель, боюсь возникнет путаница, если продолжу писать.
Почему здесь столько sql запросов на info_hash:
PHP:
        WHERE HEX(tor.info_hash) = '$info_hash_sql'
            OR HEX(tor.info_hash) = '$info_hash_sql2'
            OR HEX(tor.info_hash) = '$info_hash_sql3'
            OR HEX(tor.info_hash2) = '$info_hash_sql'
            OR HEX(tor.info_hash2) LIKE '$info_hash_sql%'
 
Логика следующая (если опираться на мой код):
-Я словил дубль в базе для разных торрентов в поле info_hash из-за DB()->escape() (к сожалению это было несколько лет назад, но помню что это было связано с торрентом у которого в info_hash была последовательность \\', \\"", \\" или что-то подобное)
-Я отказался от экранирования и заменил на rtrim($info_hash, ' ')
-Позже отказался и от rtrim

Поэтому вот такой велосипед из трех проверок для совместимости

четверкая проверка
OR HEX(tor.info_hash2) LIKE '$info_hash_sql%'
Расчитана на то, чтобы проверить является ли хэш обрезаной версией v2 32 битного хэша, либо полной версией

В коде движка это реализовано как SUBSTRING(tor.info_hash_v2, 1, 20)

Если этого не указать в анонсере - отваливаются старые торренты, т.к. в function tracker_register( было указано $info_hash_sql = rtrim(DB()->escape($info_hash), ' ');

Это все делалось на рабочем проекте и наблюдалось
 
Статус: Подтверждено, исправлено

Баг валидный. Проблема действительно существовала в движке.

Что было не так

Конструкция rtrim(DB()->escape($info_hash), ' ') содержала сразу две проблемы при работе с бинарными данными info_hash:

  1. DB()->escape() использует PDO::quote(), который предназначен для текстовых строк, а не для бинарных данных. Если в 20-байтовом SHA-1 хэше встречались байты 0x27 (одинарная кавычка) или 0x5c (обратный слэш), они экранировались — что меняло сами байты при сравнении с VARBINARY-колонкой. Например, байт \x27 превращался в \x5c\x27 — два байта вместо одного.
  2. rtrim($str, ' ') обрезает trailing 0x20-байты (пробел). Если последний байт SHA-1 хэша оказывался равен 0x20, он удалялся, и WHERE-условие не находило торрент. Вероятность этого — примерно 1/256 для каждого хэша, что при достаточном количестве торрентов гарантированно вызовет проблемы.

Как исправлено

Во всех местах, где info_hash использовался в raw SQL-запросах, мы заменили rtrim(DB()->escape($info_hash), ' ') на подход с UNHEX(bin2hex()):

PHP:
// Было:
$info_hash_sql = rtrim(DB()->escape($info_hash), ' ');
$where = "WHERE info_hash = '{$info_hash_sql}'";

// Стало:
$info_hash_hex = bin2hex($info_hash);
$where = "WHERE info_hash = UNHEX('{$info_hash_hex}')";

Почему это безопасно и корректно:
  • bin2hex() преобразует бинарные данные в hex-строку, содержащую только символы [0-9a-f] — никакого экранирования не требуется
  • UNHEX() на стороне MySQL/MariaDB преобразует hex обратно в бинарные данные для побайтового сравнения с VARBINARY-колонкой
  • UNHEX() — стандартная функция MySQL (с версии 4.1) и MariaDB, не является специфичной для какой-либо СУБД
  • Никакие байты не теряются и не модифицируются

Затронутые файлы

  • app/Http/Controllers/Tracker/AnnounceController.php — поиск торрента при announce
  • app/Http/Controllers/Tracker/ScrapeController.php — поиск при scrape (IN-clause)
  • src/Torrent/Registry.php — проверка дубликатов при регистрации торрента
  • library/includes/functions.php — поиск по info_hash из админки

Также добавлен индекс на колонку info_hash в таблице bb_bt_torrents для ускорения поиска.

Тесты

Написаны интеграционные тесты (24 теста), которые проверяют корректность на реальной MySQL-базе, включая все edge-case байты: 0x00, 0x20, 0x27, 0x5c, полностью нулевые хэши, полностью пробельные хэши, а также последовательность \x5c\x27 (backslash + quote).

Один из тестов подтверждает, что старый подход с rtrim действительно не находит торрент, у которого хэш заканчивается на 0x20.

PR: fix(tracker): replace rtrim(escape()) with UNHEX(bin2hex()) for binary info_hash queries by exileum · Pull Request #2338 · torrentpier/torrentpier
 
Back
Top