to the top
close form
Для получения триального ключа
заполните форму ниже
Team license
Enterprise license
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

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

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

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

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

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

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


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

>
>
>
Проверка Bitcoin

Проверка Bitcoin

29 Июл 2014

Ничего эпического в этой статье не будет. Мы проверили с помощью PVS-Studio исходный код Bitcoin. Нашли всего пару подозрительных мест. Это не удивительно. Думаю, эти исходные коды не проверял только ленивый. Но раз проверили, то решил написать маленькую заметку. Так сказать, "для галочки".

0268_Bitcoin_ru/image1.png

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

  • Надо найти проекты, которые собираются с помощью GCC, но при этом их можно собрать с помощью Clang. Если мы будем проверять проекты, которые уже ориентированы на Clang, это будет нечестно. Clang в них естественно ничего не найдёт, так как ошибки уж поправлены. А PVS-Studio найдёт.
  • Анализатору PVS-Studio приходится играть на чужом поле под названием "Linux". Почти нет проектов, которые можно одновременно собрать с помощью Clang и Visual Studio. Теоретически говорится, что Clang уже хорошо совместим с Visual Studio. На практике это неправда. Не удаётся собрать и проверить многие проекты. PVS-Studio в свою очередь плохо проверяет проекты в Linux. В результате приходится выбирать проекты, с которыми одинаково удачно могут работать оба инструмента.

Одним из проектов, выбранных для сравнения стал Bitcoin. Оба анализатора в нём почти ничего не нашли. Это не удивительно. Думаю, этот проект проверялся уже многими инструментами. Соответственно, этот проект, скорее всего, не войдёт в сравнение. Поэтому, пусть останется хотя бы эта краткая заметка.

Анализ проекта

Описывать, что такое Bitcoin не нужно. Исходные коды взяты с помощью:

git clone https://github.com/bitcoin/bitcoin.git

Анализ производился с помощью PVS-Studio версии 5.17.

Странный цикл

Нашлось только одно место, интересное на мой взгляд. Это какая-то функция, относящаяся к криптографии. Что она делает я не знаю. Возможно, я нашел EPIC FAIL. Сейчас модно находить эпические ошибки, связанные с безопасностью. Однако скорее всего, это мелкая ошибка или даже вовсе, так и было задумано.

bool CCryptoKeyStore::Unlock(const CKeyingMaterial& vMasterKeyIn)
{
  {
    LOCK(cs_KeyStore);
    if (!SetCrypted())
      return false;

    CryptedKeyMap::const_iterator mi = mapCryptedKeys.begin();
    for (; mi != mapCryptedKeys.end(); ++mi)
    {
      const CPubKey &vchPubKey = (*mi).second.first;
      const std::vector<unsigned char> &vchCryptedSecret =
        (*mi).second.second;
      CKeyingMaterial vchSecret;
      if(!DecryptSecret(vMasterKeyIn, vchCryptedSecret,
                        vchPubKey.GetHash(), vchSecret))
          return false;
      if (vchSecret.size() != 32)
          return false;
      CKey key;
      key.Set(vchSecret.begin(), vchSecret.end(),
              vchPubKey.IsCompressed());
      if (key.GetPubKey() == vchPubKey)
          break;
      return false;
    }
    vMasterKey = vMasterKeyIn;
  }
  NotifyStatusChanged(this);
  return true;
}

Предупреждение PVS-Studio: V612 An unconditional 'return' within a loop. crypter.cpp 169

Обратите внимание на цикл. В нем должны перебираться какие-то ключи. Но тело цикла выполняется только один раз. В конце цикла расположен оператор "return false;". Цикл также, может быть прерван досрочно с помощью операторов "break;". При этом в теле цикла нет ни одного оператора "continue;".

Подозрительный сдвиг

static int64_t set_vch(const std::vector<unsigned char>& vch)
{
  if (vch.empty())
    return 0;

  int64_t result = 0;
  for (size_t i = 0; i != vch.size(); ++i)
      result |= static_cast<int64_t>(vch[i]) << 8*i;

  // If the input vector's most significant byte is 0x80,
  // remove it from the result's msb and return a negative.
  if (vch.back() & 0x80)
      return -(result & ~(0x80 << (8 * (vch.size() - 1))));

   return result;
}

Предупреждение PVS-Studio: V629 Consider inspecting the '0x80 << (8 * (vch.size() - 1))' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. script.h 169

Функция формирует 64-битное значение. Один сдвиг сделан правильно, а второй возможно нет.

Правильно:

static_cast<int64_t>(vch[i]) << 8*i

В начале переменная расширяется до int64_t и только потом происходит сдвиг.

Подозрительно:

0x80 << (8 * (vch.size() - 1))

Константа 0x80 имеет тип 'int'. Это значит, что при сдвиге может произойти переполнение. Так как функция генерирует 64-битное значение, то я подозреваю наличие ошибки. Подробнее про сдвиги я писал в статье "Не зная брода, не лезь в воду - часть третья".

Безопасный вариант:

0x80ull << (8 * (vch.size() - 1))

Опасные классы

class CKey {
  ....
  // Copy constructor. This is necessary because of memlocking.
  CKey(const CKey &secret) : fValid(secret.fValid),
                             fCompressed(secret.fCompressed) {
    LockObject(vch);
    memcpy(vch, secret.vch, sizeof(vch));
  }
  ....
};

Предупреждение PVS-Studio: V690 The 'CKey' class implements a copy constructor, but lacks the '=' operator. It is dangerous to use such a class. key.h 175

Как следует из текста комментария, конструктор копирования нужен для осуществления синхронизации. Однако, скопировать объект можно не только с помощью конструктора копирования, но и с помощью operator =. А этот оператор не реализован. Даже если сейчас operator = нигде не используется это всё равно потенциально опасно.

Аналогично, стоит обратить внимание ещё на несколько классов:

  • V690 The 'Semantic_actions' class implements the '=' operator, but lacks a copy constructor. It is dangerous to use such a class. json_spirit_reader_template.h 196
  • V690 The 'CFeeRate' class implements a copy constructor, but lacks the '=' operator. It is dangerous to use such a class. core.h 118
  • V690 The 'CTransaction' class implements the '=' operator, but lacks a copy constructor. It is dangerous to use such a class. core.h 212
  • V690 The 'CTxMemPoolEntry' class implements a copy constructor, but lacks the '=' operator. It is dangerous to use such a class. txmempool.h 27
  • V690 The 'Json_grammer' class implements the '=' operator, but lacks a copy constructor. It is dangerous to use such a class. json_spirit_reader_template.h 370
  • V690 The 'Generator' class implements the '=' operator, but lacks a copy constructor. It is dangerous to use such a class. json_spirit_writer_template.h 98

Заключение

Регулярное использование статических анализаторов может сэкономить массу времени и нервных клеток. Главное, чтобы это было удобно. Например, попробуйте инкрементальный анализ в PVS-Studio. Просто пишешь код и если что, тебя остановят. К хорошему быстро привыкаешь.

Популярные статьи по теме
Под капотом SAST: как инструменты анализа кода ищут дефекты безопасности

Дата: 26 Янв 2023

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

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

Дата: 17 Янв 2023

Автор: Гость

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

Дата: 29 Дек 2022

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

Дело идёт к Новому году, а значит, самое время традиционно вспомнить десять самых интересных срабатываний, которые нашёл PVS-Studio в 2022 году.
PVS-Studio и RPCS3: лучшие предупреждения в один клик

Дата: 12 Дек 2022

Автор: Александр Куренев

Best Warnings — режим анализатора, оставляющий в окне вывода 10 лучших предупреждений. Мы предлагаем вам ознакомиться с обновлённым режимом Best Warnings на примере проверки проекта RPCS3.
Holy C++

Дата: 23 Ноя 2022

Автор: Гость

В этой статье постараюсь затронуть все вещи, которые можно без зазрения совести выкинуть из С++, не потеряв ничего (кроме боли), уменьшить стандарт, нагрузку на создателей компиляторов, студентов, из…

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

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