PVS-Studio теперь в Chocolatey: проверка Chocolatey из-под Azure DevOps

• Об использовании Chocolatey
• Настройка
• Результаты проверки
• Заключение
• P.S.

Мы продолжаем делать использование PVS-Studio удобнее. Теперь наш анализатор доступен в Chocolatey, пакетном менеджере для Windows. Мы полагаем, что это облегчит развёртывание PVS-Studio, в частности, в облачных сервисах. Чтобы не идти далеко, проверим исходный код всё того же Chocolatey. В качестве CI системы выступит Azure DevOps.

Вот список других наших статей на тему интеграции с облачными системами:

• PVS-Studio идёт в облака: Azure DevOps
• PVS-Studio идёт в облака: Travis CI
• PVS-Studio идёт в облака: CircleCI
• PVS-Studio идёт в облака: GitLab CI/CD

Советую обратить внимание на первую статью про интеграцию с Azure DevOps, так как в данном случае некоторые моменты опущены, чтобы не дублироваться.

Итак, герои данной статьи:

PVS-Studio — инструмент статического анализа кода, предназначенный для выявления ошибок и потенциальных уязвимостей в программах, написанных на языках С, C++, C# и Java. Работает в 64-битных системах на Windows, Linux и macOS, и может анализировать код, предназначенный для 32-битных, 64-битных и встраиваемых ARM платформ. Если вы впервые будете пробовать статический анализ кода для проверки своих проектов, то рекомендуем ознакомиться со статьёй о том, как быстро посмотреть самые интересные предупреждения PVS-Studio и оценить возможности этого инструмента.

Azure DevOps - набор облачных сервисов, совместно охватывающих весь процесс разработки. В состав данной платформы входят такие инструменты, как Azure Pipelines, Azure Boards, Azure Artifacts, Azure Repos, Azure Test Plans, позволяющие ускорить процесс создания программного обеспечения и повысить его качество.

Chocolatey – пакетный менеджер для Windows с открытым исходным кодом. Цель проекта - автоматизировать весь жизненный цикл программного обеспечения от установки до обновления и удаления в операционных системах Windows.

Об использовании Chocolatey

Посмотреть, как установить сам пакетный менеджер, вы можете по этой ссылке. Полная документация по установке анализатора доступна по ссылке в разделе "Установка с использованием пакетного менеджера Chocolatey". Вкратце повторю некоторые моменты оттуда.

Команда для установки последней версии анализатора:

choco install pvs-studio

Команда установки конкретной версии пакета PVS-Studio:

choco install pvs-studio --version=7.05.35617.2075

По умолчанию устанавливается только ядро анализатора - компонент Core. Все остальные флаги (Standalone, JavaCore, IDEA, MSVS2010, MSVS2012, MSVS2013, MSVS2015, MSVS2017, MSVS2019) можно передать при помощи ‑‑package-parameters.

Пример команды, которая установит анализатор с плагином для Visual Studio 2019:

choco install pvs-studio --package-parameters="'/MSVS2019'"

Теперь посмотрим на пример удобного использования анализатора под Azure DevOps.

Настройка

Напоминаю, что про такие моменты, как регистрация учётной записи, создание Build Pipeline и синхронизация учётной записи с проектом, лежащим в репозитории на GitHub, есть отдельная статья. Наша же настройка сразу начнётся с написания конфигурационного файла.

Для начала настроим триггер запуска, указав, что производим запуск только для изменений в master ветке:

trigger:
- master

Далее нам нужно выбрать виртуальную машину. На данный момент это будет Microsoft-hosted агент с Windows Server 2019 и Visual Studio 2019:

pool:
  vmImage: 'windows-latest'

Перейдём к телу конфигурационного файла (блок steps). Несмотря на то, что в виртуальную машину нельзя установить произвольное ПО, я не стал добавлять Docker контейнер. Мы можем добавить Chocolatey как расширение для Azure DevOps. Для этого перейдём по ссылке. Жмём Get it free. Далее, если вы уже авторизированы, просто выбираем свою учётную запись, а если нет, то проделываем всё то же самое после авторизации.

Тут нужно выбрать, куда мы добавим расширение, и нажать кнопку Install.

После благополучной установки нажмём Proceed to organization:

Теперь можно увидеть шаблон для задачи Chocolatey в окне tasks при редактировании конфигурационного файла azure-pipelines.yml:

Нажмём на Chocolatey и увидим список полей:

Здесь нам нужно выбрать install в поле с командами. В Nuspec File Name укажем название нужного пакета – pvs-studio. Если не указать версию, установится последняя, что нас полностью устраивает. Нажмём на кнопку add и увидим сформировавшуюся задачу в файле конфигурации.

steps:
- task: ChocolateyCommand@0
  inputs:
    command: 'install'
    installPackageId: 'pvs-studio'

Далее перейдём к основной части нашего файла:

- task: CmdLine@2
  inputs:
    script:

Теперь нам нужно создать файл с лицензией анализатора. Здесь PVSNAME и PVSKEY – названия переменных, значения которых мы указываем в настройках. Они будут хранить логин и лицензионный ключ PVS-Studio. Чтобы установить их значения, откроем меню Variables->New variable. Создадим переменные PVSNAME для логина и PVSKEY для ключа анализатора. Не забудьте поставить галочку Keep this value secret для PVSKEY. Код команды:

сall "C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe" credentials 
–u $(PVSNAME) –n $(PVSKEY)

Соберём проект при помощи bat-файла, лежащего в репозитории:

сall build.bat

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

сall mkdir PVSTestResults

Запустим анализ проекта:

сall "C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe" 
–t .\src\chocolatey.sln –o .\PVSTestResults\Choco.plog

Конвертируем наш отчёт в формат html при помощи утилиты PlogСonverter:

сall "C:\Program Files (x86)\PVS-Studio\PlogConverter.exe" 
–t html –o \PVSTestResults\ .\PVSTestResults\Choco.plog

Теперь нужно создать задание для того, чтобы можно было выгрузить отчёт.

- task: PublishBuildArtifacts@1
  inputs:
    pathToPublish: PVSTestResults
    artifactName: PVSTestResults
    condition: always()

Полный файл конфигурации выглядит так:

trigger:
- master

pool:
  vmImage: 'windows-latest'

steps:
- task: ChocolateyCommand@0
  inputs:
    command: 'install'
    installPackageId: 'pvs-studio'

- task: CmdLine@2
  inputs:
    script: |
      call "C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe" 
      credentials –u $(PVSNAME) –n $(PVSKEY)
      call build.bat
      call mkdir PVSTestResults
      call "C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe" 
      –t .\src\chocolatey.sln –o .\PVSTestResults\Choco.plog
      call "C:\Program Files (x86)\PVS-Studio\PlogConverter.exe" 
      –t html –o .\PVSTestResults\ .\PVSTestResults\Choco.plog

- task: PublishBuildArtifacts@1
  inputs:
    pathToPublish: PVSTestResults
    artifactName: PVSTestResults
    condition: always()

Нажмём Save->Save->Run для запуска задачи. Выгрузим отчёт, зайдя во вкладку задачи.

Проект Chocolatey содержит всего 37615 строк C# кода. Рассмотрим некоторые из найденных ошибок.

Результаты проверки

Предупреждение N1

Предупреждение анализатора: V3005 The 'Provider' variable is assigned to itself. CrytpoHashProviderSpecs.cs 38

public abstract class CrytpoHashProviderSpecsBase : TinySpec
{
  ....
  protected CryptoHashProvider Provider;
  ....
  public override void Context()
  {
    Provider = Provider = new CryptoHashProvider(FileSystem.Object);
  }
}

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

Предупреждение N2

Предупреждение анализатора: V3093 [CWE-480] The '&' operator evaluates both operands. Perhaps a short-circuit '&&' operator should be used instead. Platform.cs 64

public static PlatformType get_platform()
{
  switch (Environment.OSVersion.Platform)
  {
    case PlatformID.MacOSX:
    {
      ....
    }
    case PlatformID.Unix:
    if(file_system.directory_exists("/Applications")
      & file_system.directory_exists("/System")
      & file_system.directory_exists("/Users")
      & file_system.directory_exists("/Volumes"))
      {
        return PlatformType.Mac;
      }
        else
          return PlatformType.Linux;
    default:
      return PlatformType.Windows;
  }
}

Отличие оператора & от оператора && заключается в том, что если левая часть выражения - false, то всё равно будет вычислена правая часть, что в данном случае подразумевает лишние вызовы метода system.directory_exists.

В рассмотренном фрагменте это мелкий недочёт. Да, данное условие можно оптимизировать, заменив оператор & на оператор &&, но, с практической точки зрения, это ни на что не влияет. Однако, в других случаях путаница между & и && может вызывать серьезные проблемы, когда правая часть выражения будет работать с некорректными/недопустимыми значения. Например, в нашей коллекции ошибок, выявленных с помощью диагностики V3093, есть вот такой случай:

if ((k < nct) & (s[k] != 0.0))

Даже если индекс k некорректен, он будет использоваться для доступа к элементу массива. В результате будет сгенерировано исключение IndexOutOfRangeException.

Предупреждения N3, N4

Предупреждение анализатора: V3022 [CWE-571] Expression 'shortPrompt' is always true. InteractivePrompt.cs 101

Предупреждение анализатора: V3022 [CWE-571] Expression 'shortPrompt' is always true. InteractivePrompt.cs 105

public static string 
prompt_for_confirmation(.... bool shortPrompt = false, ....)
{
  ....
  if (shortPrompt)
  {
    var choicePrompt = choice.is_equal_to(defaultChoice) //1
    ?
    shortPrompt //2
    ?
    "[[{0}]{1}]".format_with(choice.Substring(0, 1).ToUpperInvariant(), //3
    choice.Substring(1,choice.Length - 1))
    :
    "[{0}]".format_with(choice.ToUpperInvariant()) //0
    : 
    shortPrompt //4
    ? 
    "[{0}]{1}".format_with(choice.Substring(0,1).ToUpperInvariant(), //5
    choice.Substring(1,choice.Length - 1)) 
    :
    choice; //0
    ....
  }
  ....
}

В данном случае имеет место быть странная логика работы тернарного оператора. Рассмотрим подробнее: если выполнится условие, помеченное мной цифрой 1, то мы перейдём к условию 2, которое всегда true, а значит выполнится строчка 3. Если же условие 1 окажется ложным, то мы перейдём на строчку, помеченную цифрой 4, условие в которой тоже всегда true, а значит, выполнится строчка 5. Таким образом, условия, помеченные комментарием 0, никогда не будут выполнены, что может являться не совсем той логикой работы, на которую рассчитывал программист.

Предупреждение N5

Предупреждение анализатора: 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 condition. Options.cs 1019

private static string GetArgumentName (...., string description)
{
  string[] nameStart;
  if (maxIndex == 1)
  {
    nameStart = new string[]{"{0:", "{"};
  }
  else
  {
    nameStart = new string[]{"{" + index + ":"};
  }
  for (int i = 0; i < nameStart.Length; ++i) 
  {
    int start, j = 0;
    do 
    {
      start = description.IndexOf (nameStart [i], j);
    } 
    while (start >= 0 && j != 0 ? description [j++ - 1] == '{' : false);
    ....
    return maxIndex == 1 ? "VALUE" : "VALUE" + (index + 1);
  }
}

Диагностика сработала на строку:

while (start >= 0 && j != 0 ? description [j++ - 1] == '{' : false)

Так как переменная j на несколько строк выше инициализируется нулём, тернарный оператор вернёт значение false. Из-за данного условия, тело цикла выполнится только один раз. Мне кажется, что данный фрагмент кода работает совсем не так, как задумывал программист.

Предупреждение N6

Предупреждение анализатора: V3022 [CWE-571] Expression 'installedPackageVersions.Count != 1' is always true. NuGetService.cs 1405

private void remove_nuget_cache_for_package(....)
{
  if (!config.AllVersions && installedPackageVersions.Count > 1)
  {
    const string allVersionsChoice = "All versions";
    if (installedPackageVersions.Count != 1)
    {
      choices.Add(allVersionsChoice);
    }
    ....
  }
  ....
}

Здесь странное вложенное условие: installedPackageVersions.Count != 1, которое всегда будет true. Часто такое предупреждение указывает на логическую ошибку в коде, а в остальных случаях просто на избыточную проверку.

Предупреждение N7

Предупреждение анализатора: V3001 There are identical sub-expressions 'commandArguments.contains("-apikey")' to the left and to the right of the '||' operator. ArgumentsUtility.cs 42

public static bool arguments_contain_sensitive_information(string
 commandArguments)
{
  return commandArguments.contains("-install-arguments-sensitive")
  || commandArguments.contains("-package-parameters-sensitive")
  || commandArguments.contains("apikey ")
  || commandArguments.contains("config ")
  || commandArguments.contains("push ")
  || commandArguments.contains("-p ")
  || commandArguments.contains("-p=")
  || commandArguments.contains("-password")
  || commandArguments.contains("-cp ")
  || commandArguments.contains("-cp=")
  || commandArguments.contains("-certpassword")
  || commandArguments.contains("-k ")
  || commandArguments.contains("-k=")
  || commandArguments.contains("-key ")
  || commandArguments.contains("-key=")
  || commandArguments.contains("-apikey")
  || commandArguments.contains("-api-key")
  || commandArguments.contains("-apikey")
  || commandArguments.contains("-api-key");
}

Программист, который написал данный участок кода, скопипастил две последние строчки и забыл их отредактировать. Из-за этого пользователи Chocolatey лишились возможности применить параметр apikey ещё парой способов. Аналогично параметрам выше, могу предложить такие варианты:

commandArguments.contains("-apikey=");
commandArguments.contains("-api-key=");

Copy-paste ошибки имеют большой шанс рано или поздно появиться в любом проекте с большим количеством исходного кода, и одно из лучших средств борьбы с ними – статический анализ.

P.S. И как всегда, эта ошибка тяготеет появиться в конце многострочного условия :). См. публикацию "Эффект последней строки".

Предупреждение N8

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

public virtual ConcurrentDictionary<string, PackageResult> get_outdated(....)
{
  ....
  var pinnedPackageResult = outdatedPackages.GetOrAdd(
    packageName, 
    new PackageResult(installedPackage, 
                      _fileSystem.combine_paths(
                        ApplicationParameters.PackagesLocation, 
                        installedPackage.Id)));
  ....
  if (   installedPackage != null
      && !string.IsNullOrWhiteSpace(installedPackage.Version.SpecialVersion) 
      && !config.UpgradeCommand.ExcludePrerelease)
  {
    ....
  }
  ....
}

Классическая ошибка: вначале объект installedPackage используется, а потом проверяется на null. Данная диагностика говорит нам об одной из двух проблем в программе: либо installedPackage никогда не равен null, что сомнительно, и тогда проверка избыточна, либо мы потенциально можем получить серьёзную ошибку в коде – попытку доступа по нулевой ссылке.

Заключение

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

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

P.S.

Перед публикацией мы отправили статью разработчикам Сhocolatey, и они хорошо её приняли. Ничего критичного нами найдено не было, но им, например, понравилась найденная нами ошибка, связанная с ключом "api-key".