Добро пожаловать на Pro Pawn - Портал о PAWN-скриптинге.
Показано с 1 по 10 из 10
  1. #1
    Аватар для Daniel_Cortez
    "Это не хак, это фича"

    Статус
    Оффлайн
    Регистрация
    06.04.2013
    Адрес
    Novokuznetsk, Russia
    Сообщений
    2,192
    Репутация:
    2589 ±

    Баг записи строк в функциях SA-MP

    Вместо предисловия

    Начиналось всё обыденно, я изучал принцип работы функции SHA256_PassHash, как вдруг случайно заметил, что если размер массива меньше длины строки с хешем, строка будет записана не совсем правильно.
      Открыть/закрыть

    1. // В массив str1 будет записываться строка с хеш-суммой.
    2. new str1[4];
    3.  
    4. // Все элементы массива str2 с самого начала равны не нулю, а 0xFFFFFFFF.
    5. new str2[4] = { 0xFFFFFFFF, ... };
    6.  
    7. main()
    8. {
    9. // Вся строка не вместится в массив str1, она должна быть урезана
    10. // до 3 символов (4-м будет символ конца строки '\0'),
    11. // а все элементы массива str2 должны остаться равны 0xFFFFFFFF.
    12. SHA256_PassHash("", "", str1, sizeof(str1));
    13.  
    14. printf("str1: \"%s\"", str1);
    15. for (new i = 0; i < sizeof(str1); ++i)
    16. printf("str1[%02d]: %08x", i, str1[i]);
    17. for (new i = 0; i < sizeof(str2); ++i)
    18. printf("str2[%02d]: %08x", i, str2[i]);
    19. }


    Вывод:
    Код:
    str1: "E3B0"
    str1[00]: 00000045
    str1[01]: 00000033
    str1[02]: 00000042
    str1[03]: 00000030
    str2[00]: 00000000
    str2[01]: FFFFFFFF
    str2[02]: FFFFFFFF
    str2[03]: FFFFFFFF
    Как видно из вывода, строка была "урезана", чтобы вместиться в массив из 4 ячеек, вот только вместо 3 символов функция записала 4, а завершающий '\0' записался в 0-ю ячейку массива str2, располагающегося в памяти сразу после str1. Иными словами, это выход за пределы массива. Соль в том, что обычно такой баг очень трудно отловить: попробуй догадайся, из-за чего обнуляется какая-то рандомная переменная - точно не подумаешь на одну из функций SA-MP.


    Нахождение и разбор

    Казалось бы, ничего особенного, обычный мелкий баг - в SA-MP десятки, если не сотни таких багов, которые никогда не будут исправлены.
    Но любопытство взяло верх: решил проверить, получится ли добиться чего-то похожего от GetWeaponName() - и там оказался аналогичный эффект при урезании строки.

    Чтобы разобраться в этом баге, нужно иметь доступ к исходным кодам SA-MP - для этого как раз может подойти Open-SAMP, основанный на утекших исходниках (предположительно от версии 0.3a).
    Рассмотрим реализацию GetWeaponName():
    1. cell AMX_NATIVE_CALL funcGetWeaponName ( AMX* a_AmxInterface, cell* a_Params )
    2. {
    3. if(bScriptDebug) logprintf ( "[Call]-> funcGetWeaponName()" );
    4. CHECK_PARAMS( 3 );
    5.  
    6. if(a_Params[1] > WEAPON_COLLISION) return 0;
    7. return set_amxstring(a_AmxInterface, a_Params[2], __NetGame->getWeaponName(a_Params[1]), a_Params[3]);
    8. }

    Те, кто знакомы с написанием плагинов для SA-MP, могут знать, что для записи сишных строк в массив Pawn нужно использовать функцию amx_SetSrting(). Однако в коде выше можно заметить, что строка с названием оружия записывается в массив с помощью "самопальной" функции set_amxstring().
    Рассмотрим эту функцию:
    1. int set_amxstring(AMX* amx, cell amx_addr, const char* source, int max)
    2. {
    3. cell* dest = (cell *)(amx->base + (int)(((AMX_HEADER *)amx->base)->dat + amx_addr));
    4. cell* start = dest;
    5. while (max--&&*source)
    6. *dest++=(cell)*source++;
    7. *dest = 0;
    8. return dest-start;
    9. }

    Присмотритесь внимательно к циклу while, в нём и находится причина бага: параметр max изначально равен размеру массива, но запись символов строки происходит не max-1, а max раз - отсюда запись одного лишнего символа там, где по идее должен быть завершающий '\0', и запись '\0' в следующую ячейку, уже за пределами массива. Если в условии цикла заменить max-- на --max, баг будет исправлен.


    Подверженные функции

    Как удалось выяснить, данному багу подвержены все нативные функции SA-MP, которые записывают строки (т.е. все, что используют set_amxstring()).
      Открыть/закрыть

    • GetPlayerIp
    • SHA256_PassHash
    • GetAnimationName
    • GetWeaponName
    • NetStats_GetIpPort
    • GetPlayerName
    • gpci
    • GetPlayerVersion
    • GetServerVarAsString/GetConsoleVarAsString
    • GetPVarString
    • GetPVarNameAtIndex
    • GetSVarString
    • GetSVarNameAtIndex
    • db_field_name
    • db_get_field
    • db_get_field_assoc


    Исключение: format() - в этой функции выхода за пределы массива не происходит. Очевидно, баг обнаружили ранее (всё-таки функция часто используется) и сообщили Kalcor'у, но тот не стал вникать в детали, искать и исправлять первопричину бага в set_amxstring() (иначе баг был бы исправлен не только в format(), но и во всех остальных функциях), а просто в коде format() вместо самопальной set_amxstring() использовал стандартную amx_SetString() - в этом можно убедиться, дизассемблировав сервер версии 0.3a (именно в этой версии баг был исправен). А то, что первопричина бага может быть глубже и затрагивает другие функции - да кого это волнует? Главное же, что баг нашли только в format(), и теперь он там исправлен. В общем, кое-что да говорит о подходе к разработке SA-MP (впрочем, ничего нового).


    Предпринятые меры:

    • Информация о баге и способе исправления передана 0x452, проблема устранена в предстоящем релизе RW-MP.
    • Пытаться сообщить о баге Kalcor'у? Не смешите. Этот баг не вызывает краш сервера и не мешает считать деньги с Hosted'а, а потому не достоен внимания Солнцеликого.



    Автор: Daniel_Cortez

    Специально для Pro-Pawn.ru
    Копирование данной статьи на других ресурсах без разрешения автора запрещено!
    Индивидуально в ЛС по скриптингу не помогаю. Задавайте все свои вопросы здесь (click).

  2. 8 пользователя(ей) сказали cпасибо:
    0x452 (07.05.2018) Battista (07.05.2018) Elrmrnt-Kritik (07.05.2018) Kovshevoy (07.05.2018) Nurick (07.05.2018) pawnoholic (09.05.2018) VVWVV (07.05.2018) Web (08.05.2018)
  3. #2
    Аватар для Daniel_Cortez
    "Это не хак, это фича"

    Статус
    Оффлайн
    Регистрация
    06.04.2013
    Адрес
    Novokuznetsk, Russia
    Сообщений
    2,192
    Репутация:
    2589 ±
    И да, для фанбоев и тех, кто просто любит спорить ради спора:
    Ничего ты не понимаешь! Это не баг, параметр размера в этих функциях называется не "size" а "length", поэтому нужно указывать "sizeof(<массив>) - 1"!
    • Это не интуитивно и такое мог придумать только полный идиот или изврат (нужное подчеркнуть).
    • В стандартных функциях Pawn размер массива по умолчанию задаётся с помощью sizeof, без всяких "-1", хотя параметр размера тоже назван "maxlength":
      1. native fread(File: handle, string[], size = sizeof string, bool: pack = false);
      2. native strpack(dest[], const source[], maxlength=sizeof dest);
      3. native strunpack(dest[], const source[], maxlength=sizeof dest);
      4. native strcopy(dest[], const source[], maxlength=sizeof dest);
      5. native strcat(dest[], const source[], maxlength=sizeof dest);

    • Этот баг, хоть и индивидуально, но был исправлен в format().
    • В SA-MP wiki в примерах указывается именно "sizeof(<массив>)", без "-1". Если бы это было неправильно, те люди, что приближены к процессу разработки SA-MP (на самого Kalcor'а глупо надеяться, по очевидным причинам), давно бы заметили и исправили это.
    Индивидуально в ЛС по скриптингу не помогаю. Задавайте все свои вопросы здесь (click).

  4. #3
    Аватар для Nexius_Tailer
    Пользователь

    Статус
    Оффлайн
    Регистрация
    04.01.2015
    Адрес
    Гомель, Беларусь
    Сообщений
    547
    Репутация:
    158 ±
    Хорошо, что такое начинают сообщать разработчикам сторонних мп на основе сампа, так хоть какая-то практическая польза от этого есть.

    - - - Добавлено - - -

    P.s. Только непонятно, почему это "урок" и чему он должен обучать.
    Не хотите постоянно проверять обновления моих скриптов?
    Подключите его последним, после всех остальных
    Nexius's Update Checker

  5. #4
    Аватар для Daniel_Cortez
    "Это не хак, это фича"

    Статус
    Оффлайн
    Регистрация
    06.04.2013
    Адрес
    Novokuznetsk, Russia
    Сообщений
    2,192
    Репутация:
    2589 ±
    Цитата Сообщение от Nexius_Tailer Посмотреть сообщение
    P.s. Только непонятно, почему это "урок" и чему он должен обучать.
    Странно, почему никто об этом ничего не сказал в аналогичной теме про баг в GetPVarString/GetSVarString.

    Как бы то ни было, в обеих темах сменил префикс на [Info]. Заодно восстановил отдельный пост для спорщиков, который скрыл вчера.
    Индивидуально в ЛС по скриптингу не помогаю. Задавайте все свои вопросы здесь (click).

  6. #5
    Аватар для Geebrox
    Пользователь

    Статус
    Оффлайн
    Регистрация
    24.08.2015
    Адрес
    Ташкент
    Сообщений
    375
    Репутация:
    97 ±
    Я этот баг замечал когда делал соль для паролей аккаунтов, пердавал размер массива в 65 символа, то есть MAX_HASH_SIZE = 65 использую sizeof. В БД сохранялся хеш из 65 символов, не знал из за чего именно происходило это. Далее изменил значение константы MAX_HASH_SIZE на 64 и создавал массивы с MAX_HASH_SIZE + 1; а в функции SHA256_PassHash вместо sizeof начал использовать саму константу.

  7. #6
    Аватар для Nexius_Tailer
    Пользователь

    Статус
    Оффлайн
    Регистрация
    04.01.2015
    Адрес
    Гомель, Беларусь
    Сообщений
    547
    Репутация:
    158 ±
    Цитата Сообщение от Daniel_Cortez Посмотреть сообщение
    Странно, почему никто об этом ничего не сказал в аналогичной теме про баг в GetPVarString/GetSVarString.

    Как бы то ни было, в обеих темах сменил префикс на [Info]. Заодно восстановил отдельный пост для спорщиков, который скрыл вчера.
    Наверное никто не обратил внимания.
    Можно, кстати, для статей подобного рода отдельный раздел запилить.
    Последний раз редактировалось Nexius_Tailer; 07.05.2018 в 22:57.
    Не хотите постоянно проверять обновления моих скриптов?
    Подключите его последним, после всех остальных
    Nexius's Update Checker

  8. #7
    Аватар для ziggi
    Проверенный

    Статус
    Оффлайн
    Регистрация
    14.05.2015
    Сообщений
    1,181
    Репутация:
    790 ±
    Цитата Сообщение от Nexius_Tailer Посмотреть сообщение
    Можно, кстати, для статей подобного рода отдельный раздел запилить.
    Раздел с багами сампа? Не надо, а то размер БД форума превысит лимиты хостинга

  9. 5 пользователя(ей) сказали cпасибо:
    #Djuga (09.05.2018) 0x452 (07.05.2018) Kovshevoy (08.05.2018) Nexius_Tailer (07.05.2018) Web (08.05.2018)
  10. #8
    Аватар для Daniel_Cortez
    "Это не хак, это фича"

    Статус
    Оффлайн
    Регистрация
    06.04.2013
    Адрес
    Novokuznetsk, Russia
    Сообщений
    2,192
    Репутация:
    2589 ±
    Цитата Сообщение от Geebrox Посмотреть сообщение
    Я этот баг замечал когда делал соль для паролей аккаунтов, пердавал размер массива в 65 символа, то есть MAX_HASH_SIZE = 65 использую sizeof. В БД сохранялся хеш из 65 символов, не знал из за чего именно происходило это. Далее изменил значение константы MAX_HASH_SIZE на 64 и создавал массивы с MAX_HASH_SIZE + 1; а в функции SHA256_PassHash вместо sizeof начал использовать саму константу.
    Немного оффтопа по именованию константы:
      Открыть/закрыть
    • Что означает твоё название "MAX_HASH_SIZE": длина строки с абсолютно любым хешем или только с тем, который выдаёт SHA256_PassHash()? Если второе, то по идее константу следует называть "MAX_SHA256_HASH_SIZE".
    • Длина строки и её размер - разные вещи; размер больше длины на 1 (место под завершающий символ '\0'). Если я правильно понял, ты в своей константе хранишь именно длину строки - тогда почему в названии "SIZE", а не "LEN"?
    • В инклудах SA-MP есть константа "MAX_PLAYER_NAME" - возможно, лучше принять тот же стиль, без "LEN" или "SIZE" в конце названия. (На самом деле спорное утверждение, поскольку есть ещё "MAX_CHATBUBBLE_LENGTH", но я думаю, что более короткий вариант всё же проще и лучше.)

    С учётом вышеперечисленных замечаний хорошим названием будет "MAX_SHA256_HASH" - кстати, именно так я и назвал константу длины хеша в своей реализации SHA256_PassHash(), которую не так давно предложил 0x452 к включению в следующий релиз RW-MP.



    Цитата Сообщение от Nexius_Tailer Посмотреть сообщение
    Можно, кстати, для статей подобного рода отдельный раздел запилить.
    Цитата Сообщение от ziggi Посмотреть сообщение
    Раздел с багами сампа? Не надо, а то размер БД форума превысит лимиты хостинга
    Шутки шутками, но в самом Pawn багов тоже немало и стагнирует он не хуже мультиплеера.

    Касаемо раздела, после открытия wiki в нём отпадёт всякая необходимость.
    К слову, до открытия осталось не так уж и долго, я почти подготовил "изначальный запас" статей по стандартным нативкам и нескольким функциям SA-MP.
    Индивидуально в ЛС по скриптингу не помогаю. Задавайте все свои вопросы здесь (click).

  11. Пользователь сказал cпасибо:
    Geebrox (08.05.2018)
  12. #9
    Аватар для Geebrox
    Пользователь

    Статус
    Оффлайн
    Регистрация
    24.08.2015
    Адрес
    Ташкент
    Сообщений
    375
    Репутация:
    97 ±
    Цитата Сообщение от Daniel_Cortez Посмотреть сообщение
    Длина строки и её размер - разные вещи; размер больше длины на 1 (место под завершающий символ '\0').
    Да, зная это я изначально назначал константе значение - 65. Учту Ваши советы, спасибо.

  13. #10
    Аватар для Daniel_Cortez
    "Это не хак, это фича"

    Статус
    Оффлайн
    Регистрация
    06.04.2013
    Адрес
    Novokuznetsk, Russia
    Сообщений
    2,192
    Репутация:
    2589 ±
    Ещё одно интересное проявление бага:
    1. SomeFunc()
    2. {
    3. SetSVarString("-", "abcd");
    4. new str[3];
    5. GetSVarString("-", str, sizeof(str));
    6. }
    7.  
    8. main()
    9. {
    10. new str[] = "abcd";
    11. printf("str = \"%s\"", str);
    12. printf("FRM = %d", emit(lctrl 5));
    13. SomeFunc();
    14. printf("FRM = %d", emit(lctrl 5));
    15. printf("str = \"%s\"", str);
    16. }

    Код:
    str = "abcd"
    FRM = 16580
    FRM = 0
    str = "♀,¶Y0-"
    Последствия в этом случае могут быть куда хуже, чем при непреднамеренном обнулении переменной, объявленной до/после массива.
    Если до локального массива не было объявлено ни одной переменной, лишний '\0' перезапишет ту ячейку, в которой хранится значение регистра FRM для предыдущей функции - т.е. при возврате из SomeFunc() в main() значение FRM окажется обнулено (из-за этого в выводе строка "FRM = 0").
    Будучи обнулённым, регистр FRM указывает на начало секции данных. А поскольку смещения локальных переменных относительно FRM являются отрицательными, обращение к локальным переменным на самом деле будут происходить не к области стека, а по отрицательным адресам (0 - <смещение>) , т.е. к тому, что находится до секции данных - к секции кода. Именно поэтому в последнем printf() в коде выше выводится каша из бессвязных символов - это не данные, а код (инструкции AMX). Если же вместо чтения из локальной переменной попытаться записать в неё данные - либо интерпретатор отловит операцию записи по невалидному адресу и поднимет ошибку времени выполнения, либо, если запись происходит с помощью нативной функции (например, получение ника с помощью GetPlayerName()), будет перезаписано содержимое секции кода, после чего может произойти буквально всё что угодно.

    TL;DR: Поистине адский баг.
    Индивидуально в ЛС по скриптингу не помогаю. Задавайте все свои вопросы здесь (click).

  14. Пользователь сказал cпасибо:
    DeimoS (09.08.2018)
 

 

Информация о теме

Пользователи, просматривающие эту тему

Эту тему просматривают: 1 (пользователей: 0 , гостей: 1)

Ваши права

  • Вы не можете создавать новые темы
  • Вы не можете отвечать в темах
  • Вы не можете прикреплять вложения
  • Вы не можете редактировать свои сообщения
  •