Для получения триального ключа
заполните форму ниже
Team License (базовая версия)
Enterprise License (расширенная версия)
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

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

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

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

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

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

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


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

>
>
>
SARIF SDK и его ошибки

SARIF SDK и его ошибки

10 Дек 2019

Сегодня у нас на тесте очередной качественный проект Microsoft, в котором мы всё же попытаемся героически поискать ошибки при помощи PVS-Studio. SARIF – аббревиатура от "Static Analysis Results Interchange Format", представляет собой стандарт (формат файла), предназначенный для взаимодействия и обмена результатами работы статических анализаторов с другими инструментами: IDE, комплексными инструментами проверки и анализа кода (например, SonarQube), системами непрерывной интеграции и т.п. SARIF SDK, соответственно, содержит инструментарий разработчика .NET для поддержки SARIF, а также вспомогательные файлы.

0694_sarif_sdk_ru/image1.png

SARIF возник в Microsoft и теперь является стандартом, разрабатываемым в рамках OASIS (некоммерческий консорциум, который занимается открытыми стандартами). SARIF предназначен для передачи не только результатов работы анализатора, но и метаданных об инструменте, а также данных о том, как он был запущен, временных метках и так далее. Более подробно со стандартом можно ознакомиться на сайте OASIS. Исходный код SARIF SDK можно скачать с репозитория на GiHub. Домашняя страница проекта доступна по ссылке.

О проекте

Проект SARIF SDK оказался маленьким: 799 файлов .cs (примерно 98 000 непустых строк кода). В составе проекта содержатся тесты, которые я всегда исключаю из проверки. Таким образом, интересующая нас часть кода составила 642 файла .cs (примерно 79 000 непустых строк кода). Это, конечно, мало. Зато проверка и анализ прошли легко и быстро, между делом, что я и попытался отразить на картинке в начале статьи. Тем не менее, кое-какие интересные ошибки найти удалось. Давайте на них посмотрим.

Ошибки

V3070 [CWE-457] Uninitialized variable 'Binary' is used when initializing the 'Default' variable. MimeType.cs 90

public static class MimeType
{
  ....
  /// <summary>The MIME type to use when no better MIME type is known.</summary>
  public static readonly string Default = Binary;
  ....
  /// <summary>The MIME type for binaries.</summary>
  public static readonly string Binary = "application/octet-stream";
  ....
}

Поле Default инициализируют значением другого поля, которое еще не получило значения. В результате, Default получит значение по умолчанию для типа string – null. Вероятно, ошибку не заметили, потому что поле Default нигде не используют. Но всё может измениться, и тогда разработчик столкнётся с непредвиденным результатом или падением программы.

V3061 Parameter 'logicalLocationToIndexMap' is always rewritten in method body before being used. PrereleaseCompatibilityTransformer.cs 1963

private static JArray ConvertLogicalLocationsDictionaryToArray(
  ....
  Dictionary<LogicalLocation, int> logicalLocationToIndexMap,
  ....)
{
  ....
  logicalLocationToIndexMap =
    new Dictionary<LogicalLocation, int>(LogicalLocation.ValueComparer);
  ....
}

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

private static bool ApplyChangesFromTC25ThroughTC30(....)
{
  ....
  Dictionary<LogicalLocation, int> logicalLocationToIndexMap = null;
  ....
  logicalLocationToIndexMap =
    new Dictionary<LogicalLocation, int>(LogicalLocation.ValueComparer);

  run["logicalLocations"] =
    ConvertLogicalLocationsDictionaryToArray(
      ....,
      logicalLocationToIndexMap,
      ....);
}

Странный и подозрительный код.

V3008 [CWE-563] The 'run.Tool' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 116, 114. ExportRulesMetadataCommandBase.cs 116

public partial class Run
{
  ....
  public Tool Tool { get; set; }
  ....
}

public partial class Tool : ....
{
  ....
  public Tool()
  {
  }
  ....
}

private void OutputSarifRulesMetada(....)
{
  ....
  var run = new Run();
  run.Tool = new Tool();

  run.Tool = Tool.CreateFromAssemblyData(....);  // <=
  ....
}

Свойству run.Tool дважды присваивают значение. Как при создании объекта Tool, так и при записи в свойство Tool, никакой дополнительной работы не производится. Поэтому повторное присваивание выглядит подозрительно.

V3042 [CWE-476] Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'loc' object WhereComparer.cs 152

private static Uri ArtifactUri(ArtifactLocation loc, Run run)
{
  return loc?.Uri ?? loc.Resolve(run)?.Uri;
}

В случае, если значение переменной loc окажется равным null, будет произведена попытка возврата значения из правой части оператора ??, что приведет к доступу по нулевой ссылке.

V3042 [CWE-476] Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'formatString' object InsertOptionalDataVisitor.cs 194

public override Message VisitMessage(Message node)
{
  ....
  node.Text = node.Arguments?.Count > 0
    ? string.Format(...., formatString.Text, ....)
    : formatString?.Text;
  ....
}

В двух параллельных ветках условного оператора ?: используют небезопасный и безопасный варианты доступа по потенциально нулевой ссылке formatString.

V3042 [CWE-476] Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'messageText' object FortifyFprConverter.cs 1210

V3042 [CWE-476] Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'messageText' object FortifyFprConverter.cs 1216

private void AddMessagesToResult(Result result)
{
  ....
  string messageText = (rule.ShortDescription ?? rule.FullDescription)?.Text;
  ....
  if (....)
  {
      // Replace the token with an embedded hyperlink.
      messageText = messageText.Replace(....);
  }
  else
  {
      // Replace the token with plain text.
      messageText = messageText.Replace(....);
  }
  ....
}

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

V3080 [CWE-476] Possible null dereference. Consider inspecting 'fileDataVersionOne.Uri'. SarifCurrentToVersionOneVisitor.cs 1030

private IDictionary<string, FileDataVersionOne>
  CreateFileDataVersionOneDictionary()
{
  ....
  FileDataVersionOne fileDataVersionOne = CreateFileDataVersionOne(v2File);

  if (fileDataVersionOne.Uri.OriginalString.Equals(key))
  {
    ....
  }
  ....
}

Анализатор заподозрил, что возможен NullReferenceException при работе со ссылкой fileDataVersionOne.Uri. Посмотрим, откуда приходит эта переменная, и прав ли анализатор. Для этого изучим тело метода CreateFileDataVersionOne:


private FileDataVersionOne CreateFileDataVersionOne(Artifact v2FileData)
{  
  FileDataVersionOne fileData = null;

  if (v2FileData != null)
  {
    ....
    fileData = new FileDataVersionOne
    {
      ....
      Uri = v2FileData.Location?.Uri,
      ....
    };
    ....
  }

  return fileData;
}

public partial class FileDataVersionOne
{
  ....
  public Uri Uri { get; set; }
  ....
}

Действительно, при создании объекта класса FileDataVersionOne свойство Uri может получить значение null. Хороший пример совместной работы механизмов анализа потока данных и межпроцедурного анализа.

V3080 [CWE-476] Possible null dereference. Consider inspecting '_jsonTextWriter'. SarifLogger.cs 242

public virtual void Dispose()
{
  ....
  if (_closeWriterOnDispose)
  {
    if (_textWriter != null) { _textWriter.Dispose(); }
    if (_jsonTextWriter == null) { _jsonTextWriter.Close(); }  // <=
  }
  ....
}

Здесь допущена опечатка. Очевидно, что в условии второго блока if должно быть _jsonTextWriter != null. Опасность данного кода в том, что он, скорее всего, не падает, потому что _jsonTextWriter как раз не равен null. Но при этом и поток остается открытым.

V3083 [CWE-367] Unsafe invocation of event 'RuleRead', NullReferenceException is possible. Consider assigning event to a local variable before invoking it. FxCopConverter.cs 897

private void ReadRule(....)
{
  ....
  if (RuleRead != null)
  {
    RuleRead(....);
  }
  ....
}

С событием работают небезопасно. Некритичная ошибка, которую легко исправить, например, следуя подсказке Visual Studio. Вот такую замену предлагает IDE:

private void ReadRule(....)
{
  ....
  RuleRead?.Invoke(....);
  ....
}

Работы на несколько секунд, зато анализатор больше не ругается, а IDE не подсвечивает код. Ещё одна подобная ошибка:

  • V3083 [CWE-367] Unsafe invocation of event 'ResultRead', NullReferenceException is possible. Consider assigning event to a local variable before invoking it. FxCopConverter.cs 813

V3095 [CWE-476] The 'v1Location' object was used before it was verified against null. Check lines: 333, 335. SarifVersionOneToCurrentVisitor.cs 333

internal Location CreateLocation(LocationVersionOne v1Location)
{
  ....
  string key = v1Location.LogicalLocationKey ??
                v1Location.FullyQualifiedLogicalName;

  if (v1Location != null)
  {
    ....
  }
  ....
}

Автор кода предполагал, что ссылка v1Location может быть нулевой, поэтому добавил соответствующую проверку. При этом в коде чуть выше он почему-то работает с данной ссылкой безо всяких проверок. Невнимательность при рефакторинге? Трудно сказать.

V3125 [CWE-476] The 'v1StackFrame' object was used after it was verified against null. Check lines: 1182, 1171. SarifVersionOneToCurrentVisitor.cs 1182

internal StackFrame CreateStackFrame(StackFrameVersionOne v1StackFrame)
{
  StackFrame stackFrame = null;

  if (v1StackFrame != null)
  {
    stackFrame = new StackFrame
    {
      ....
    };
  }

  stackFrame.Location =
    CreateLocation(v1StackFrame.FullyQualifiedLogicalName,
                   v1StackFrame.LogicalLocationKey,
                   ....);

  return stackFrame;
}

Традиционно – обратная ситуация. Сначала ссылку v1StackFrame проверяют на равенство null, а затем о проверке забыли. Но здесь есть один важный нюанс: переменные v1StackFrame и stackFrame логически связаны. Смотрите, если v1StackFrame будет = null, то объект StackFrame не будет создан, а stackFrame так и останется = null. При этом программа упадет на вызове stackFrame.Location, так как никаких проверок тут нет. То есть до опасного использования v1StackFrame, на которое указал анализатор, дело даже не дойдет. Этот код работоспособен только в случае передачи в метод CreateStackFrame ненулевых значений v1StackFrame. Я заподозрил, что в вызывающем коде это как-то контролируется. Вызовы CreateStackFrame выглядят так:

Frames = v1Stack.Frames?.Select(CreateStackFrame).ToList()

CreateStackFrame используют как селектор. Проверки передаваемых ссылок на равенство null здесь нет. Возможно, где-то при заполнении коллекции Frames это (запись нулевых ссылок) контролируется, но так далеко копать я не стал. Вывод и так очевиден - код требует внимания авторов.

Заключение

Статья получилась небольшая, зато никто не успел заскучать :) Напоминаю, что вы всегда можете скачать наш анализатор, чтобы самостоятельно поискать ошибки в своих или чужих проектах.

И напоследок, небольшой анонс: следующая моя статья будет про самые интересные ошибки, которые я и мои коллеги нашли в C# проектах в 2019 году. Следите за нашим блогом. До встречи!

Чтобы узнать о новых публикациях в блоге, вы можете подписаться на следующие каналы:

Популярные статьи по теме
Главный вопрос программирования, рефакторинга и всего такого

Дата: 14 Апр 2016

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

Вы угадали, ответ - "42". Здесь приводится 42 рекомендации по программированию, которые помогут избежать множества ошибок, сэкономить время и нервы. Автором рекомендаций выступает Андрей Карпов - тех…
Эффект последней строки

Дата: 31 Май 2014

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

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

Дата: 22 Дек 2018

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

В канун празднования нового 2019 года команда PVS-Studio решила сделать приятный подарок всем контрибьюторам open-source проектов, хостящихся на GitHub, GitLab или Bitbucket. Им предоставляется возмо…
Любите статический анализ кода!

Дата: 16 Окт 2017

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

Я в шоке от возможностей статического анализа кода, хотя сам участвую в разработке инструмента PVS-Studio. На днях я был искренне удивлён тому, что анализатор оказался умнее и внимательнее меня.
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний

Дата: 31 Июл 2017

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

После большой статьи про проверку операционной системы Tizen мне было задано много вопросов о проценте ложных срабатываний и о плотности ошибок (сколько ошибок PVS-Studio выявляет на 1000 строк кода)…
Статический анализ как часть процесса разработки Unreal Engine

Дата: 27 Июн 2017

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

Проект Unreal Engine развивается - добавляется новый код и изменятся уже написанный. Неизбежное следствие развития проекта - появление в коде новых ошибок, которые желательно выявлять как можно раньш…
PVS-Studio для Java

Дата: 17 Янв 2019

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

В седьмой версии статического анализатора PVS-Studio мы добавили поддержку языка Java. Пришло время немного рассказать, как мы начинали делать поддержку языка Java, что у нас получилось и какие дальн…
PVS-Studio ROI

Дата: 30 Янв 2019

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

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

Дата: 19 Май 2017

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

Возможно, читатели помнят мою статью под названием "Эффект последней строки". В ней идёт речь о замеченной мной закономерности: ошибка чаще всего допускается в последней строке однотипных блоков текс…
Технологии, используемые в анализаторе кода PVS-Studio для поиска ошибок и потенциальных уязвимостей

Дата: 21 Ноя 2018

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

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

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

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