Мои аплодисменты проекту Tor

12 Май 2017

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

Поздравляю авторов проекта Tor. Я не смог найти в этом проекте ошибок, проверив его с помощью статического анализатора PVS-Studio. Такие слова после проверки программ я пишу очень редко, и авторам проекта можно гордиться собой. Вручаю медальку за качественный код.

Это не первая проверка проекта Tor с помощью анализатора PVS-Studio. Предыдущая проверка состоялась в 2012 году, и тогда я написал небольшую заметку: "Безопасность, безопасность! А вы её тестируете?". Что интересно, 5 лет назад я мог найти в проекте ошибки с помощью PVS-Studio, а сейчас не могу.

За прошедшие 5 лет мы существенно развили диагностические возможности PVS-Studio и он научился выявлять новые паттерны ошибок, такие как - утечки памяти, мертвый код, неправильную работу со строками типа BSTR и много другое. Я легко нахожу с помощью анализатора PVS-Studio ошибки в коде таких проектов, как GCC, LLVM, C++ REST SDK, GDB, Qt, Chromium, Linux kernel (см. список статей). Но сесть и написать статью про ошибки, найденные в Tor, я не смог!

О чем это говорит? Авторы проекта начали очень тщательно и серьезно относиться к качеству и безопасности кода.

Были случаи, когда в начале мы не находили ошибки, а затем, когда анализатор становился мощнее, начинали находить ошибки. Но это, кажется, впервые, когда мы раньше находили ошибки, а теперь не находим. Разработчики Tor молодцы! Вот образец для подражания другим программистам.

Пара слов о самой проверке. Проверялись самые свежие на момент написания статьи исходные коды проекта Tor. Для анализа использовался PVS-Studio версии 6.15. Анализатор выдаёт некоторое количество сообщений, но все они являются ложными или не несут пользы. По итогам изучения отчёта, я выписал 9 паттернов ложных срабатываний и со временем мы доработаем анализатор. Так что, скорее, получилось, что это была не проверка проекта Tor, а дополнительное тестирование PVS-Studio :).

Как я уже сказал, есть сообщения, которые вроде и не совсем ложные, но и толку от них никакого нет. Простой пример:

ssize_t read_all(...., size_t count, ....)
{
  ....
  if (count > SIZE_T_CEILING || count > SSIZE_MAX) {

Анализатор выдаёт предупреждение: V590 Consider inspecting this expression. The expression is excessive or contains a misprint. util.c 2116

Взглянем на строчку после раскрытия макросов:

if (count > ((size_t)(0x7fffffffffffffffL -16)) ||
    count > 0x7fffffffffffffffL) {

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

Есть предупреждения по делу, но всё равно они не тянут на написание статьи под громким названием "Баги в проекте Tor". Один из таких случаев:

const char *err = strchr(cp, ':')+2;
tor_assert(err);

Ассерт никогда не сработает. Даже если функция strchr вернёт NULL, то указатель err будет хранить хоть и не валидное значение ((char *)2), но всё равно не NULL. Правильно было бы написать так:

const char *err = strchr(cp, ':');
tor_assert(err);
err += 2;

Впрочем, как я понимаю, функция strchr никогда не вернёт NULL, и проверка написана просто на всякий случай.

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

static time_t
edge_of_accounting_period_containing(time_t now, int get_end)
{
  ....
  case UNIT_MONTH: {
    if (tm.tm_mday < cfg_start_day ||
        (tm.tm_mday < cfg_start_day && before)) {
      --tm.tm_mon;
    }
  ....
}

Предупреждение PVS-Studio: V686 A pattern was detected: (tm.tm_mday < cfg_start_day) || ((tm.tm_mday < cfg_start_day) && ...). The expression is excessive or contains a logical error. hibernate.c 333

Проверку можно сократить до:

if (tm.tm_mday < cfg_start_day) {

Вероятнее всего в этом условии допущена какая-то опечатка или логическая ошибка.

Больше мне написать нечего. Возможно, конечно, я просмотрел ещё какую-то ошибку, но общую картину это всё равно не меняет. Проверяя другие открытые проекты, я и мои коллеги выписываем ошибки десятками и сотнями. Сейчас в нашей коллекции уже собрано более 10000 ошибок, найденных нами в различных проектах. В проекте Tor я бы тоже с радостью их нашел, а потом описал в статье 20-30 интересных ошибок, но не смог. Ещё раз хвалю авторов этого проекта.