w23 commented 3 years ago

Второй аргумент этой функции указывает на буфер, в который будет скопирована строка. При этом, размер этого буфера нигде не указывается, поэтому крайне легко нечаянно создать карту, в которой где-то в недрах будет токен, вылезающий за размер этого буфера. (мы на это наткнулись в vk/rtx форке)

Увы, не вижу, как можно это исправить без изменения сигнатуры функции:

либо добавить аргумент с размером буфера (предпочтительно)
либо пускай аллоцирует и возвращает результат нужного размера (ну, такое)

w23 commented 3 years ago

337 похож на вот это же

nekonomicon commented 3 years ago

Там проблема была не в размере токена, а в формате структуры не совместимом с оригинальным hlsdk на amd64. То есть проблема уже не актуальна и тот багрепорт нужно закрыть. По аналогии с этим: https://github.com/FWGS/xash3d-fwgs/issues/385 https://github.com/FWGS/hlsdk-xash3d/issues/167 https://github.com/FWGS/hlsdk-xash3d/issues/150

крайне легко нечаянно создать карту, в которой где-то в недрах будет токен, вылезающий за размер этого буфера

Я максимум поверю в то, что карту специально сломают ripent'ом. Ну и функция времён Кармака, в GoldSource она используется в неизмененном виде и так же вызывается из движка в HLSDK.

w23 commented 3 years ago

А, ох, если это часть "ABI", то, наверное, остаётся только страдать. (Но вообще, конечно, я могу просто для своих нужд написать свой собственный парсер с тем и этим, и не ходить в двигло вообще)

nekonomicon commented 3 years ago

Лучше привести пример карты на которой будет падать актуальная версия движка и уже отталкиваться от этого. А то пример с копированием мусора из неинициализированной области памяти в токен и последующей передачей его в функцию не даёт представления о потенциальной проблеме.

w23 commented 3 years ago

Оно падает не в самом движке, а в моей функции, которая зовёт COM_ParseFile на worldmodel entities и выводом в стандартный string буфер из 256 байт. На вот этой карте: test_randmap1.bsp из архива test_maps8.zip здесь https://github.com/w23/xash3d-fwgs/issues/33#issuecomment-930266601

~Фикс~ воркэраунд -- увеличить размер буфера: https://github.com/w23/xash3d-fwgs/pull/95/commits/1405196a0caec01028f7f2888c986d8a3035c4c6

a1batross commented 3 years ago

Он действительно небезопасен.

Но к сожалению, его использование размазано по всему движку и игровому коду.

Есть смысл завести версию с учетом размера второго буфера и использовать там, куда мы его можем прокинуть.

a1batross commented 3 years ago

@w23 увы так везде это и фиксится -- завести буфер побольше. :(

Если смотреть на парсер entities в сервере, там вроде 4К массив.

mittorn commented 3 years ago

Он безопасен если ты точно знаешь что размер строки в которой ищется токен не больше размера буффера под токен, иное использование небезопасно. Возможно стоит добавить вариант с указанием размера и в случае больших исходных строк использовать его

FWGS / xash3d-fwgs

COM_ParseFile is unsafe #627

337 похож на вот это же