Чтение индекса из файла является уязвимостью переполнения буфера? - PullRequest
Новая
       33

Чтение индекса из файла является уязвимостью переполнения буфера?

3 голосов
/ 11 июля 2019

Недавно я использовал инструмент статического анализа (Checkmarx) для сканирования исходного кода старого игрового движка, чтобы увидеть, есть ли в нем какие-либо уязвимости переполнения буфера.Я был удивлен, увидев, что следующий код был помечен как возможный источник переполнения буфера: 

// Get a pointer to a file that describes a 3D model
std::string filename = "my_3D_model.obj"
FILE* stream;
fopen_s(&stream, filename.c_str(), "rb");

// Read the number of vertices that make up the 3D model
int numVertices = 0;
fread(&numVertices, sizeof(int), 1, stream);

// Read the vertices and store them in a vector
// The static analysis tool doesn't complain about the use of numVertices to
// reserve space and to read from the file
std::vector<Vertex> vertices;
vertices.reserve(numVertices);
fread(vertices.data(), sizeof(Vertex), numVertices, stream);

// ...

// Copy the vertices from the vector to an array that has been allocated on the heap
// The static analysis tool complains because numVertices, which was read from a file,
// is being used as an index
Vertex* meshVertices = new Vertex[numVertices];
for (int i = 0; i < numVertices; i++)
{
    meshVertices[i] = vertices[i];
}

Инструмент статического анализа называет это «Индекс из-за уязвимости переполнения входного буфера».Он видит, что int i находится в диапазоне от 0 до numVertices, который был прочитан из файла, и думает, что это может вызвать переполнение буфера.Но возможно ли это в данном конкретном случае?numVertices используется для распределения размера буферов, поэтому я не вижу, как может произойти переполнение буфера.И если это возможно, как бы вы это предотвратили?Обратите внимание, что я не могу изменить типы буферов, потому что это сломало бы слишком много кода.

Спасибо за любую информацию!

1 Ответ

3 голосов
/ 11 июля 2019

Предупреждение абсолютно верно.Вы читаете подписанный int из внешнего источника и затем переводите его в size_t, когда звоните reserve и fread.Поскольку size_t является типом без знака, если значение, которое вы читаете из файла, является отрицательным числом, результирующее значение при повышении до size_t будет намного больше, чем абсолютное значение numVertices на большинстве платформ.В результате вы попытаетесь зарезервировать и прочитать огромное количество векторов.Если эти две операции завершатся успешно, вы попытаетесь new иметь отрицательный размер массива.Ваш цикл for гарантированно никогда не будет выполнен, если вы зайдете так далеко.

Исправление заключается в том, чтобы прочитать значение как беззнаковое целое или, что еще лучше, size_t, хотя для этого потребуется изменитькод, который записывает значение.Другой вариант - как минимум проверить значение.Доверие к данным из внешних источников - отличный способ стать подвигом на этой неделе.

...