metrica
Мы используем куки, чтобы пользоваться сайтом было удобно.
Хорошо
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 и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

Вебинар: Трудности при интеграции SAST, как с ними справляться - 04.04

>
>
>
Начало коллекционирования ошибок в функ…

Начало коллекционирования ошибок в функциях копирования

04 Апр 2020

Уже несколько раз замечал, что программисты допускают ошибки в простых функциях копирования данных. Эта тема потребует в будущем ещё много времени для изучения и подбора материала, чтобы написать основательную статью. Но захотелось поделиться парой примеров, недавно замеченных мною.

0723_copy_functions_ru/image1.png

Феномен Баадера-Майнхоф? Нет, не думаю

Как член команды PVS-Studio я сталкиваюсь с большим количеством ошибок, обнаруживаемых нами в различных проектах. Как DevRel - люблю про это рассказывать :). Сегодня я решил поговорить про неправильно реализованные функции копирования данных.

Такие неудачные функции попадались мне уже не раз. Но я их не выписывал, так как не придавал этому значения. Однако раз я заметил такую тенденцию, пора начинать их коллекционировать. Для начала поделюсь двумя последними замеченными случаями.

Кто-то может возразить, что два случая - это ещё не закономерность. И что, возможно, я обратил на них внимание исключительно из-за того, что они встретились мне через небольшое количество времени и сработал феномен Баадера-Майнхоф.

Феномен Баадера-Майнхоф (англ. the Baader-Meinhof phenomenon), также иллюзия частотности - это когнитивное искажение, при котором недавно узнанная информация, появляющаяся вновь спустя непродолжительный период времени, воспринимается как необычайно часто повторяющаяся.

Думаю, что это не так. У меня уже был опыт подобного наблюдения про функции сравнения, который затем подтверждался собранным материалом: "Зло живёт в функциях сравнения".

Ладно, перейдём к сути. Вступление для того, чтобы пока привести всего два примера, слишком затянулось :).

Пример N1

В статье про проверку Zephyr RTOS я описал вот такую неудачную попытку реализации аналога функции strdup:

static char *mntpt_prepare(char *mntpt)
{
  char *cpy_mntpt;

  cpy_mntpt = k_malloc(strlen(mntpt) + 1);
  if (cpy_mntpt) {
    ((u8_t *)mntpt)[strlen(mntpt)] = '\0';
    memcpy(cpy_mntpt, mntpt, strlen(mntpt));
  }
  return cpy_mntpt;
}

Предупреждение PVS-Studio: V575 [CWE-628] The 'memcpy' function doesn't copy the whole string. Use 'strcpy / strcpy_s' function to preserve terminal null. shell.c 427

Анализатор сообщает, что функция memcpy копирует строчку, но не скопирует терминальный ноль, и это очень подозрительно. Кажется, что этот терминальный 0 копируется здесь:

((u8_t *)mntpt)[strlen(mntpt)] = '\0';

Нет, здесь опечатка, из-за которой терминальный ноль копируется сам в себя. Обратите внимание, что запись происходит в массив mntpt, а не в cpy_mntpt. В итоге функция mntpt_prepare возвращает строку, незавершенную терминальным нулём.

На самом деле, программист хотел написать так:

((u8_t *)cpy_mntpt)[strlen(mntpt)] = '\0';

Непонятно только, зачем код написан так запутанно и нестандартно. Как результат, в небольшой и несложной функции допущена серьезная ошибка. Этот код можно упростить до следующего варианта:

static char *mntpt_prepare(char *mntpt)
{
  char *cpy_mntpt;

  cpy_mntpt = k_malloc(strlen(mntpt) + 1);
  if (cpy_mntpt) {
    strcpy(cpy_mntpt, mntpt);
  }
  return cpy_mntpt;
}

Пример N2

void myMemCpy(void *dest, void *src, size_t n) 
{ 
   char *csrc = (char *)src; 
   char *cdest = (char *)dest; 
   for (int i=0; i<n; i++) 
     cdest[i] = csrc[i]; 
}

Этот код не мы сами выявили с помощью PVS-Studio, а я случайно встретил его на сайте Stack Overflow: C and static Code analysis: Is this safer than memcpy?

Впрочем, если проверить эту функцию с помощью анализатора PVS-Studio, он справедливо заметит:

  • V104 Implicit conversion of 'i' to memsize type in an arithmetic expression: i < n test.cpp 26
  • V108 Incorrect index type: cdest[not a memsize-type]. Use memsize type instead. test.cpp 27
  • V108 Incorrect index type: csrc[not a memsize-type]. Use memsize type instead. test.cpp 27

И действительно, этот код содержит недостаток, про который указали и в ответах на Stack Overflow. Нельзя использовать в качестве индекса переменную типа int. В 64-битной программе, почти наверняка (экзотические архитектуры не рассматриваем), переменная int будет 32-битной и функция сможет скопировать не более INT_MAX байт. Т.е. не более 2 Гигабайт.

При большем размере копируемого буфера произойдёт переполнение знаковой переменной, что с точки зрения языка C и C++ является неопределённым поведением. И, кстати, не старайтесь угадать, как именно проявит себя ошибка. Это на самом деле непростая тема, про которую можно прочитать в статье "Undefined behavior ближе, чем вы думаете".

Особенно забавно, что этот код появился как попытка убрать какое-то предупреждение анализатора Checkmarx, возникавшее при вызове функции memcpy. Программист не придумал ничего лучше, как сделать свой собственный велосипед. И несмотря на простоту функции копирования, она всё равно получилась неправильной. То есть по факту человек, скорее всего, сделал ещё хуже, чем было. Вместо того, чтобы разобраться в причине предупреждения, он маскировал проблему написанием своей собственной функции (запутал анализатор). Плюс добавил ошибку, используя для счётчика int. Ах да, такой код ещё может помешать оптимизации. Неэффективно использовать свой собственный код вместо эффективной оптимизированной функции memcpy. Не делайте так :)

Заключение

Что же, я только в начале пути и, возможно, пройдёт не один год, прежде чем я накоплю материалов для основательной публикации по этой теме. Собственно, только теперь я начну выписывать подобные случаи. Спасибо за внимание и посмотрите, что интересного найдёт анализатор PVS-Studio в вашем C/C++/C#/Java коде.

Популярные статьи по теме


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

Следующие комментарии next comments
close comment form