to the top
close form

Заполните форму в два простых шага ниже:

Ваши контактные данные:

Шаг 1
Поздравляем! У вас есть промокод!

Тип желаемой лицензии:

Шаг 2
Team license
Enterprise license
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности
close form
Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Бесплатная лицензия PVS-Studio для специалистов Microsoft MVP
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Мне интересно попробовать плагин на:
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
check circle
Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

>
>
>
Красивая ошибка в реализации функции ко…

Красивая ошибка в реализации функции конкатенации строк

22 Июл 2021

У нас, разработчиков статического анализатора кода PVS-Studio, специфическое представление о красоте. О красоте багов. Нам нравится находить изящество в ошибках, разбираться в них, пытаться угадать, как они появились. Сейчас как раз интересный случай, когда в коде спутались понятия длины и размера.

0845_LFortran_strcat_ru/image1.png

Ошибка из проекта LFortran

Услышав о выходе очередного CppCast на тему LFortran, мы решили проверить этот самый LFortran. Проект небольшой, поэтому не знаем, наберется ли материал для классической статьи о проверке открытого проекта. Однако на глаза сразу попала ошибка, которая заслуживает написания отдельной маленькой заметки. На наш вкус, ошибка очень милая.

В проекте LFortran есть функции для конкатенации (объединения) двух строк в новом буфере.

void _lfortran_strcat(char** s1, char** s2, char** dest)
{
    int cntr = 0;
    char trmn = '\0';
    int s1_len = strlen(*s1);
    int s2_len = strlen(*s2);
    int trmn_size = strlen(&trmn);
    char* dest_char = (char*)malloc(s1_len+s2_len+trmn_size);
    for (int i = 0; i < s1_len; i++) {
        dest_char[cntr] = (*s1)[i];
        cntr++;
    }
    for (int i = 0; i < s2_len; i++) {
        dest_char[cntr] = (*s2)[i];
        cntr++;
    }
    dest_char[cntr] = trmn;
    *dest = &(dest_char[0]);
}

Если хотите, прежде чем мы разберём этот код, вы можете сами попробовать найти ошибку. Чтобы случайно не прочитать пояснение, вставлю длинную картинку :). Есть мем "длиннокот". А у нас будет "длинноединорог" :).

0845_LFortran_strcat_ru/image2.png

Функция должна работать следующим образом. Вычисляется размер буфера, способного вместить обе объединяемые строки и терминальный ноль. Выделяется буфер, в него копируются строки и добавляется терминальный ноль. На самом деле, выделяется буфер недостаточного размера. Его размер на 1 байт меньше, чем требуется. В результате терминальный ноль будет записан уже за пределами выделенного буфера.

Программист, писавший код, увлёкся использованием функции strlen и использовал её даже для того, чтобы определить размер терминального нуля. Произошла путаница между размером объекта (терминального нуля) и длиной пустой строки. Это странный и неверный код. Но на наш взгляд, это красивая необычная ошибка.

Пояснение:

char trmn = '\0';
int trmn_size = strlen(&trmn);

Здесь символ trmn интерпретируется как пустая строка. Её длина нулевая. Соответственно, переменная trmn_size, название которой означает размер терминального нуля, всегда будет равна 0.

Нужно было не считать длину пустой строки, а посчитать с помощью оператора sizeof, сколько байт занимает терминальный символ. Правильный код:

void _lfortran_strcat(char** s1, char** s2, char** dest)
{
    int cntr = 0;
    char trmn = '\0';
    int s1_len = strlen(*s1);
    int s2_len = strlen(*s2);

    int trmn_size = sizeof(trmn);  // <=

    char* dest_char = (char*)malloc(s1_len+s2_len+trmn_size);
    for (int i = 0; i < s1_len; i++) {
        dest_char[cntr] = (*s1)[i];
        cntr++;
    }
    for (int i = 0; i < s2_len; i++) {
        dest_char[cntr] = (*s2)[i];
        cntr++;
    }
    dest_char[cntr] = trmn;
    *dest = &(dest_char[0]);
}

Обнаружение ошибки

Как уже было сказано, ошибка была найдена с помощью статического анализатора кода PVS-Studio. К сожалению, он не смог обнаружить ошибку, именно как выход за границу массива. Это сделать достаточно сложно. Анализ потока данных не смог сопоставить, как связан размер буфера dest_char и значение переменной cntr, увеличивающейся в циклах. Ошибка была обнаружена косвенным путём.

PVS-Studio выдал предупреждение: V742 [CWE-170, CERT-EXP37-C] Function receives an address of a 'char' type variable instead of pointer to a buffer. Inspect the first argument. lfortran_intrinsics.c 550

Очень странно считать длину строки с помощью strlen, передав ей указатель на одиночный символ. И действительно, в процессе изучения обнаруженная анализатором аномалия оказалась серьёзным багом. Статический анализ — это круто!

Продолжаем улучшать код

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

Во-первых, анализатору не нравится, что нет проверки указателя, который возвращает функция malloc. Это важно. Предупреждение: V522 [CWE-690, CERT-MEM52-CPP] There might be dereferencing of a potential null pointer 'dest_char'. Check lines: 553, 551. lfortran_intrinsics.c 553

Во-вторых, анализатор выдаёт несколько предупреждений на тему 64-битных ошибок. Код не готов к работе со строками, длина которых может быть больше INT_MAX символов. Понятно, что это экзотика, но всё равно некрасиво и потенциально опасно. Лучше использовать тип size_t вместо int.

Улучшенный вариант функции:

void _lfortran_strcat(const char** s1, const char** s2, char** dest)
{
    if (s1 == NULL || *s1 == NULL ||
        s2 == NULL || *s2 == NULL || dest == NULL)
    {
      // Какая-то обработка ошибки, уместная в данном проекте.
      ....
    }
    size_t cntr = 0;
    const char trmn = '\0';
    const size_t s1_len = strlen(*s1);
    const size_t s2_len = strlen(*s2);
    char* dest_char = (char*)malloc((s1_len+s2_len+1)*sizeof(char));
    if (dest_char == NULL)
    {
      // Какая-то обработка ошибки, уместная в данном проекте.
      ....
    }

    for (size_t i = 0; i < s1_len; i++) {
        dest_char[cntr] = (*s1)[i];
        cntr++;
    }
    for (size_t i = 0; i < s2_len; i++) {
        dest_char[cntr] = (*s2)[i];
        cntr++;
    }
    dest_char[cntr] = trmn;
    *dest = dest_char;
}

Новый код тоже не претендует на идеальность, но он стал явно лучше. Спасибо за внимание. И приходите попробовать PVS-Studio для проверки собственных проектов.

Дополнительные ссылки

Популярные статьи по теме
64-битные ошибки: LONG, LONG_PTR и привет из прошлого

Дата: 09 Мар 2023

Автор: Андрей Карпов

В целом, 64-битные ошибки - дело минувших дней. Мало кто сейчас занимается портированием кода с 32-битной на 64-битную систему. Кому это было нужно, уже портировали свои приложения. Кому не нужно, то…
Приключения капитана Блада: потонет ли Арабелла?

Дата: 14 Фев 2023

Автор: Владислав Столяров

Недавно в сети появилась новость о том, что был открыт исходный код игры "Приключения капитана Блада". Мы не смогли пройти мимо и проверили его качество с помощью PVS-Studio. Потонет ли легендарный к…
Тонкости C++: итак, вы объявили класс…

Дата: 07 Фев 2023

Автор: Сергей Ларин

Во время работы наша команда постоянно сталкивается с некоторыми особенностями языка, которые могут быть неизвестны рядовому C++ программисту. В этой статье мы расскажем о том, как работает, казалось…
Под капотом SAST: как инструменты анализа кода ищут дефекты безопасности

Дата: 26 Янв 2023

Автор: Сергей Васильев

Сегодня речь о том, как SAST-решения ищут дефекты безопасности. Расскажу, как разные подходы к поиску потенциальных уязвимостей дополняют друг друга, зачем нужен каждый из них и как теория ложится на…
Ложные представления программистов о неопределённом поведении

Дата: 17 Янв 2023

Автор: Гость

Неопределённое поведение (UB) – непростая концепция в языках программирования и компиляторах. Я слышал много заблуждений в том, что гарантирует компилятор при наличии UB. Это печально, но неудивитель…


Комментарии (0)

Следующие комментарии next comments
close comment form
Unicorn with delicious cookie
Мы используем куки, чтобы пользоваться сайтом было удобно.
Хорошо