Комментарии 33
Плюс функции strncpy и strncat провоцируют ошибки, ибо позволяют строке в итоге не оканчиваться на ноль.
Тут ещё и логическая ошибка, программист слишком перемудрил
strncat(a.consoleText, inputBuffer, sizeof(a.consoleText) –
strlen(a.consoleText) - 5);в третьей позиции должно быть:
Maximum number of characters to be appended.
так что sizeof(a.consoleText) было бы более чем достаточно.
Правда это не замечание для анализатора, а для кода в целом.
В случае просто sizeof() туда можно будет скопировать больше чем влезет...
Логику этого поведения мне не совсем понять (это именно сколько добавить, а не сколько всего места есть в буфере...) но таки правильно записано. ну, почти.
был неправ
https://www.cplusplus.com/reference/cstring/strncat/
/* strncat example */
#include <stdio.h>
#include <string.h>
int main ()
{
char str1[20];
char str2[20];
strcpy (str1,"To be ");
strcpy (str2,"or not to be");
strncat (str1, str2, 6);
puts (str1);
return 0;
}
Output: "To be or not"
лучше смотреть man
A simple implementation of strncat() might be:
char*
strncat(char *dest, const char *src, size_t n)
{
size_t dest_len = strlen(dest);
size_t i;
for (i = 0 ; i < n && src[i] != '\0' ; i++)
dest[dest_len + i] = src[i];
dest[dest_len + i] = '\0';
return dest;
}вот, что бывает, когда долго работаешь со фреймворками, забываешь стандартное поведение.
И чисто теоретически, strlen(a.consoleText) может вернуть любой мусор в этом случае.
Почему? strncat делает, цитирую, "Appends the first num characters of source to destination, plus a terminating null-character.". Именно из-за "plus a terminating null-character" нам и надо вычитать в третьем аргументе 1цу
А не хотите выделить подобные случаи в отдельную диагностику? "buffer overflow" тут выглядит не очень понятным, хотелось бы видеть что-то типа "третий аргумент не должен быть отрицательным" (если он отрицательный – это уже ошибка, и не надо даже смотреть, переполняется ли буфер).
А откуда появилось магическое число пять?
Возможно, можно как-то изменить предупреждение, чтобы оно напоминало про underflow в этом конкретном примере, но это будет очень сложно, и я даже не представляю, как это может работать с точки зрения пользователя.
Надо подсчитать количество подстрок в строке на С++.
Решение, которое пишут в 80% случаев:
size_t substr_count(const char *str, const char* substr)
{
size_t count = 0;
const char *p = str;
do {
p = strstr(p, substr);
}
while (p != nullptr && ++count && *(++p) != 0);
return count;
}
Тут С++ -ом даже и не пахнет, зато арифметика на сырых указателях, куча условий в цикле. И это ещё один из лучших примеров, который приходилось видеть.
А теперь как сделать тоже самое на С++:
std::size_t substr_count(std::string_view text, std::string_view sub) noexcept
{
std::size_t t_count {0};
for (std::size_t pos {0}; (pos = text.find(sub, pos)) != text.npos; ++t_count, pos += sub.length());
return t_count;
}
Даже тело цикла не требуется, и вероятность получить UB резко ниже.
А как должен быть исправлен проблемный фрагмент чтобы PVS-Studio перестал диагностировать ошибку? Перед вызовом strncat нужно сделать проверку длины строки? Типа:
if(strlen(a.consoleText)+5u < sizeof(a.consoleText))
{
strncat(a.consoleText, inputBuffer, sizeof(a.consoleText) –
strlen(a.consoleText) - 5);
}std::string concatenate(Args const&... args)
{
const size_t len = (0 + ... + std::string_view(args).size());
std::string result; result.reserve(len);
(result.append(args), ...);
return result;
}
И работать с классом std::string, если речь о С++.
После обнаружения данного срабатывания мы добавили его, как пример, в документацию V645 и там предлагаем такой вариант фикса:
size_t textSize = strlen(a.consoleText);
if (sizeof(a.consoleText) - textSize > 5)
{
strncat(a.consoleText, inputBuffer,
sizeof(a.consoleText) - textSize - 5);
}Понятно, спасибо за ответ.
sizeof(a.consoleText) — textSize > 5
Какой-нибудь gcc на высоких уровнях предупреждений ругнется на сравнение unsigned с signed. Уж извините за занудство :)
Кроме того, тут напрашивается еще вот какой момент: этот вариант можно считать надежным только в случае, если гарантируется, что в a.consoleText будет 0-символ. Если же разработчик где-то со значением consoleText ошибся и 0-символ содержится в sizeof(a.consoleText)+1u или sizeof(a.consoleText)+2, то (sizeof(a.consoleText) — textSize) приведет к переходу через ноль.
Но стоит ли этим заморачиваться, ведь отсутствие 0-символа в a.consoleText — это явно ошибочная ситуация, которая должна предотвращаться другими способами? У меня нет однозначного ответа на этот вопрос.
Один день из жизни разработчика PVS-Studio, или как я отлаживал диагностику, оказавшуюся внимательнее трёх программистов