Андрей Москалёв

18 Ноя 2025

Теги:

#CSharp

Код блокчейн-проектов Neo и NBitcoin VS анализатор кода. Кто-кого?

18 Ноя 2025

Автор: Андрей Москалёв

Введение
О PVS-Studio
Обзор потенциальных ошибок, найденных в коде Neo
Обзор потенциальных ошибок в NBitcoin
Заключение

Ввиду своей специфики блокчейн-разработка крайне требовательна к качеству кода: мало того, что незамеченная ошибка может привести к большим финансовым проблемам, она и вовсе может оказаться необратимой. Стоит ли здесь пренебрегать использованием анализатора кода? Выясним это, проверив код проектов Neo и NBitcoin.

Введение

Ранее я упомянул про специфику блокчейн-разработки. Давайте разберёмся, что конкретно подразумевалось.

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

Во-вторых, исправить код блокчейн-проекта после релиза либо проблематично (в случае кода децентрализованной сети, где необходимо, чтобы каждый из узлов согласился принять правки), либо вовсе невозможно (в случае смарт-контрактов). Зачастую также невозможно изменить и информацию, попавшую в блокчейн, в том числе и некорректную, например, полученную из-за ошибки в коде. Эти ограничения — цена за почти идеальную целостность данных.

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

Думаю, это далеко неполный список, но его вполне достаточно, чтобы сформировать представление о том, насколько велика может быть цена ошибки в коде блокчейн-проекта.

В этой статье мы рассмотрим примеры как явных, так и потенциальных ошибок, найденных с помощью статического анализатора кода PVS-Studio в двух open source C# проектах:

Neo — полноценная блокчейн-платформа, управляемая сообществом;
NBitcoin — библиотека для работы с биткоином на платформе .NET.

О PVS-Studio

PVS-Studio — инструмент для автоматического поиска потенциальных ошибок и уязвимостей безопасности в коде, также именуемый статическим анализатором кода. На момент написания статьи PVS-Studio поддерживает анализ C#, C, C++ и Java кода.

PVS-Studio используется через интеграцию с различными инструментами разработки, будь то IDE, сборочные системы, CI-сервисы или другие инструменты для проверки кода, например, SonarCube.

Для анализа Neo и NBitcoin использовался базовый подход через плагин для IDE, в нашем случае — Visual Studio. Такой плагин предоставляет возможность быстрого запуска анализа решений/проектов/отдельных файлов прямо из IDE:

А также простой и удобный интерфейс для работы с результатами анализа:

Теперь, когда мы получили общее представление об анализаторе, давайте, наконец, перейдём к обзору найденных ошибок.

Обзор потенциальных ошибок, найденных в коде Neo

Начнём с блокчейна Neo версии 3.8.2 (последняя на момент написания статьи).

Подозрительно избыточное выражение

internal static CommandStringToken Parse(...., 
                                         ref int index,
                                         ....)
{
  ....
  var ret = new CommandStringToken(....);
  index += end - index;
  return ret;
}

Предупреждение анализатора: V3107 Identical expression 'index' to the left and to the right of compound assignment. CommandStringToken.cs 80

Обратите внимание на странное выражение index += end – index. Оно эквивалентно index = index – index + end. Результатом операции вычитания всегда будет 0. Следовательно, результатом всего выражения будет значение переменной end. Получается, что вычитание избыточно. Это может быть признаком наличия ошибки, если, к примеру, вместо index должно было вычитаться какое-то другое значение.

Некорректный формат

public override string ToString()
{
  var sb = new StringBuilder();

  sb.AppendFormat("{1:X04} {2,-10}{3}{4}", 
                  Position, 
                  OpCode, 
                  DecodeOperand());

  return sb.ToString();
}

Предупреждение анализатора: V3025 [CWE-685] Incorrect format. A different number of format items is expected while calling 'AppendFormat' function. Format items not used: {3}, {4}. Arguments not used: 1st. VMInstruction.cs 105

Вызов переопределённого метода ToString неизбежно приведёт к исключению. Всё из-за некорректного вызова sb.AppendFormat, в котором допущены сразу две ошибки:

число аргументов для вставки меньше числа плейсхолдеров в строке формата, что и станет причиной исключения;
даже если первая проблема будет исправлена (число аргументов и плейсхолдеров станет равным), этот вызов всё так же будет вызывать исключение, т. к. индексация плейсхолдеров начинается с 0, а не с 1. Из этого следует, что для плейсхолдера с индексом 4 необходим 5-й аргумент, который будет отсутствовать.

Ошибка из-за путаницы в приоритетах операторов

public override int Size =>   base.Size
                            + ChangeViewMessages?.Values.GetVarSize() ?? 0
                            + 1 + PrepareRequestMessage?.Size ?? 0
                            + PreparationHash?.Size ?? 0
                            + PreparationMessages?.Values.GetVarSize() ?? 0
                            + CommitMessages?.Values.GetVarSize() ?? 0;

Предупреждение анализатора: V3123 [CWE-783] Perhaps the '??' operator works in a different way than it was expected. Its priority is lower than priority of other operators in its left part. RecoveryMessage.cs 35

На этом коде анализатор выдал сразу несколько предупреждений диагностики V3123, но для компактности я привёл только одно. Оператор ?? имеет меньший приоритет по сравнению с оператором +. Однако форматирование приведенного выражения указывает на то, что разработчик ожидал обратное.

Но есть ли в этом случае разница, какая операция будет выполнена первой? Для ответа на этот вопрос рассмотрим пример одного подвыражения сложения, если ChangeViewMessages равен null:

base.Size + ChangeViewMessages?.Values.GetVarSize() ?? 0

Неважно, какое значение будет у base.Size, результат подвыражения всегда будет 0, ведь в результате сложения base.Size c null получится всё тот же null.

Если поместить ChangeViewMessages?.Values.GetVarSize() ?? 0 в скобки, изменив тем самым порядок вычисления операций, в результатом будет base.Size.

Потенциальное null-разыменование

Issue 1

public OracleNeoFSProtocol(Wallet wallet, ECPoint[] oracles)
{
  byte[] key = oracles.Select(p => wallet.GetAccount(p))
                      .Where(p => p is not null && p.HasKey && !p.Lock)
                      .FirstOrDefault()
                      .GetKey()
                      .PrivateKey;

  privateKey = key.LoadPrivateKey();
}

Предупреждение анализатора: V3146 [CWE-476] Possible null dereference. The 'FirstOrDefault' can return default null value. OracleNeoFSProtocol.cs 37

Перед нами типичный пример потенциального null-разыменования, а именно разыменование возвращаемого значения метода FirstOfDefault, вызываемого у коллекции элементов ссылочного типа. Если коллекция окажется пуста, этот метод вернёт null, что впоследствии приведёт к выбросу исключения.

Issue 2

public bool ValidatorsChanged
{
  get
  {
    ....
    TrimmedBlock currentBlock = NativeContract.Ledger.GetTrimmedBlock(....);

    TrimmedBlock previousBlock = 
      NativeContract.Ledger.GetTrimmedBlock(...., currentBlock.Header  // <=
  .PrevHash);                                                  

    return    currentBlock.Header.NextConsensus
           != previousBlock.Header.NextConsensus;  // <=
  }
}

Предупреждения анализатора:

V3080 [CWE-476] Possible null dereference. Consider inspecting 'currentBlock'. ConsensusContext.cs 89

V3080 [CWE-476] Possible null dereference. Consider inspecting 'previousBlock'. ConsensusContext.cs 90

Анализатор дважды предупреждает о возможном null-разыменовании и указывает на переменные currentBlock и previousBlock. Но почему разыменование этих переменных опасно? Предлагаю поискать ответ в источнике значений обеих переменных — методе GetTrimmedBlock:


public TrimmedBlock GetTrimmedBlock(IReadOnlyStore snapshot, UInt256 hash)
{
  if (snapshot is null)
    throw new ArgumentNullException(nameof(snapshot));

  var key = CreateStorageKey(Prefix_Block, hash);
  
  if (snapshot.TryGet(key, out var item))
    return item.Value.AsSerializable<TrimmedBlock>();
            
  return null;
}

Метод действительно может вернуть null. Тем не менее остаётся возможность того, что это происходит в каких-то специфичных случаях. Наверняка этот момент может уточнить только автор кода, но мы можем примерно оценить вероятность, посмотрев, как часто проверяется возвращаемое значение GetTrimmedBlock на null в целом. В результате такой проверки я выяснил, что возвращаемое значение проверяется в 70% вызовов, что свидетельствует о значительном риске выброса исключения.

Issue 3

private void OnTimer(Timer timer)
{
  ....
  if (   timer.Height != context.Block.Index     // <=
      || timer.ViewNumber != context.ViewNumber)
  {
    return;
  }
      
  if (   context.Block != null                 // <=
      && context.TransactionHashes?.Length > context.Transactions?
    .Count)
   {
     ....
   }     
}

Предупреждение анализатора: V3095 [CWE-476] The 'context.Block' object was used before it was verified against null. Check lines: 173, 191. ConsensusService.cs 173

Снова предупреждение о потенциальном null-разыменовании. Свойство context.Block сначала используется без проверки, после чего проверяется на null. В лучшем случае это просто избыточная проверка. Однако может быть и так, что содержащий разыменование код является доработкой, внесённой позже базовой логики c проверкой. В этом случае разработчик вполне мог не заметить, что при выполнении этого кода свойство context.Block может иметь значение null.

Некорректный цикл

[RpcMethodWithParams]
protected internal virtual JToken GetCandidates()
{
  ....
  foreach (var item in resultstack)
  {
    var value = (Array)item;
    foreach (Struct ele in value)
    {
      var publickey = ele[0].GetSpan().ToHexString();
      json["publickey"] = publickey;
      json["votes"] = ele[1].GetInteger().ToString();

      json["active"] = validators.ToByteArray()
                                 .ToHexString()
                                 .Contains(publickey);
      jArray.Add(json);
      json = new();
    }

    return jArray;          // <=
  }
  ....
}

Предупреждение анализатора: V3020 [CWE-670] An unconditional 'return' within a loop. RpcServer.Blockchain.cs 380

Обратите внимание на возврат из метода, выполняемый в теле цикла. Ввиду отсутствия каких-либо условий и операторов continue, в этом цикле всегда будет выполняться лишь одна итерация, что почти наверняка является серьёзной ошибкой. Вероятно, была допущена опечатка, и возврат из метода должен был выполняться после цикла, а не внутри.

Опечатка в реализации метода Equals

public bool Equals(Nep11BalanceKey other)
{
  if (other is null) return false;

  if (ReferenceEquals(this, other)) return true;

  return    UserScriptHash.Equals(other.UserScriptHash) 
         && AssetScriptHash.Equals(AssetScriptHash)     // <=
         && Token.Equals(other.Token);
}

Предупреждение анализатора: V3062 An object 'AssetScriptHash' is used as an argument to its own method. Consider checking the first actual argument of the 'Equals' method. Nep11BalanceKey.cs 57

Простая, но весьма незаметная ошибка. В приведённой реализации Equals значение поля AssetScriptHash родительского объекта сравнивается само с собой. Очевидно, что вместо этого ожидалось его сравнение со значением того же поля объекта other.

Отмечу, что анализатор нашёл точно такую же ошибку в эквивалентном коде другого класса:

public bool Equals(Nep17BalanceKey other)
{
  if (other is null) return false;
  if (ReferenceEquals(this, other)) return true;
  return    UserScriptHash.Equals(other.UserScriptHash) 
         && AssetScriptHash.Equals(AssetScriptHash);
}

Обзор потенциальных ошибок в NBitcoin

Теперь давайте перейдём к обзору потенциальных ошибок в коде NBitcoin версии 9.0.1.

Ошибка из-за невнимательного copy-paste

bool SameSigHash(uint a, uint b)
{
  if (a == b)
    return true;

  if (GetTransaction() is not IHasForkId)
    return false;
  
  a = ((uint)a & ~(0x40u));
  b = ((uint)a & ~(0x40u)); // <=
  return a == b;
}

Предупреждение анализатора: V3127 [CWE-682] Two similar code fragments were found. Perhaps, this is a typo and 'b' variable should be used instead of 'a' PSBTInput.cs 948

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

Бесконечная рекурсия

public static Message ReadNext(Socket socket, 
                               Network network, 
                               uint version, 
                               CancellationToken cancellationToken, 
                               out PerformanceCounter counter)
{
  return ReadNext(socket, 
                  network, 
                  version, 
                  cancellationToken, 
                  out counter);
}

Предупреждение анализатора:

V3110 [CWE-674] Possible infinite recursion inside 'ReadNext' method. Message.cs 167

Вызов этой перегрузки метода приведёт к StackOverflowException, ведь единственное выражение в её блоке — это возврат результата её собственного рекурсивного вызова.

Одинаковые блоки в switch

public override string ToString()
{
  switch (this.Tag)
  {
    case Tags.BoolAnd:
      return "BoolAnd";
    case Tags.BoolOr:
      return "BoolAnd";   // <=
    case Tags.Add:
      return "Add";
    case Tags.Equal:
      return "Equal";
    case Tags.EqualVerify:
      return "EqualVerify";
    ....
  }
  throw new Exception("Unreachable");
}

Предупреждение анализатора: V3139 Two or more case-branches perform the same actions. ScriptToken.cs 169

Реализация метода ToString содержит switch-выражение, в котором блок одного из case дублирует блок другого. Т. к. имеется явный паттерн, подразумевающий уникальность каждого case-блока, можно с уверенностью сказать, что тут опечатка.

Перегрузка метода Equals без перегрузки GetHashCode

public class MoneyBag : IMoney, 
                        IEnumerable<IMoney>, 
                        IEquatable<MoneyBag>
{
  ....
  public bool Equals(MoneyBag other)
  {
    return Equals(other as IMoney);
  }

  public bool Equals(IMoney other)
  {
    if (other is null)
      return false;
    var m = new MoneyBag(other);
    return m._bag.SequenceEqual(_bag);
  }
  ....
}

Предупреждение анализатора: V3126 Type 'MoneyBag' implementing IEquatable<T> interface does not override 'GetHashCode' method. Money.cs 78

Анализатор обнаружил класс, реализующий интерфейс IEquatable<T>, но не переопределяющий метод GetHashCode. Это потенциально может привести к проблемам, ведь некоторые методы, например, из библиотеки Linq, в первую очередь выполняют сравнение по хэш-коду, и только если эта проверка вернёт true, то будет выполнено сравнение с помощью Equals. Если же хэши окажутся не равны, то считается, что и сами объекты тоже не равны.

Заключение

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

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

Если вы хотите опробовать PVS-Studio на собственном коде, то можете получить пробную лицензию на официальном сайте. В этом разделе документации можно найти инструкцию по её активации и ссылки на другим разделам, которые помогут вам начать работу с анализатором.

До встречи в следующих статьях!

#CSharp