PVS-Studio тепер у Chocolatey: перевірка Chocolatey з-під Azure DevOps

Ми продовжуємо використовувати 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, тому що в даному випадку деякі моменти опущені, щоби не дублюватися.

Отже, герої цієї статті:

ПВС-Студія — інструмент статичного аналізу коду, призначений виявлення помилок і потенційних уразливостей у програмах, написаних мовами З, 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, що дозволяють прискорити процес створення програмного забезпечення та підвищити його якість.

Шоколадний – пакетний менеджер для 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, є окрема стаття. Наше налаштування відразу почнеться з написання конфігураційного файлу.

Для початку налаштуємо тригер запуску, вказавши, що робимо запуск тільки для змін у майстер гілці:

trigger:
- master

Далі нам потрібно вибрати віртуальну машину. На даний момент це буде Microsoft-hosted агент з Windows Server 2019 та Visual Studio 2019:

pool:
  vmImage: 'windows-latest'

Перейдемо до тіла конфігураційного файлу (блок кроки). Незважаючи на те, що в віртуальну машину не можна встановити довільне програмне забезпечення, я не став додавати Docker контейнер. Ми можемо додати Chocolatey як розширення для Azure DevOps. Для цього перейдемо по за посиланням. Тиснемо Отримайте його безкоштовно. Далі, якщо ви вже авторизовані, просто вибираємо свій обліковий запис, а якщо ні, то проробляємо все те саме після авторизації.

Тут потрібно вибрати, куди ми додамо розширення, та натиснути кнопку Встановлювати.

Після благополучної установки натиснемо Proceed to organization:

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

Натисніть на Chocolatey і побачимо список полів:

Тут нам потрібно вибрати встановлювати у полі з командами. У Nuspec File Name вкажемо назву потрібного пакету – pvs-studio. Якщо не вказати версію, то встановиться остання, що нас повністю влаштовує. Натисніть кнопку додавати і побачимо сформоване завдання у файлі конфігурації.

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-StudioPVS-Studio_Cmd.exe" credentials 
–u $(PVSNAME) –n $(PVSKEY)

Зберемо проект за допомогою bat-файлу, що лежить у репозиторії:

сall build.bat

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

сall mkdir PVSTestResults

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

сall "C:Program Files (x86)PVS-StudioPVS-Studio_Cmd.exe" 
–t .srcchocolatey.sln –o .PVSTestResultsChoco.plog 

Конвертуємо наш звіт у формат html за допомогою утиліти PlogСonverter:

сall "C:Program Files (x86)PVS-StudioPlogConverter.exe" 
–t html –o PVSTestResults .PVSTestResultsChoco.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-StudioPVS-Studio_Cmd.exe" 
      credentials –u $(PVSNAME) –n $(PVSKEY)
      call build.bat
      call mkdir PVSTestResults
      call "C:Program Files (x86)PVS-StudioPVS-Studio_Cmd.exe" 
      –t .srcchocolatey.sln –o .PVSTestResultsChoco.plog
      call "C:Program Files (x86)PVS-StudioPlogConverter.exe" 
      –t html –o .PVSTestResults .PVSTestResultsChoco.plog

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

Натиснемо Save->Save->Run для запуску задачі. Вивантажимо звіт, зайшовши у вкладку завдання.

Проект Chocolatey містить лише 37615 рядків C# коду. Розглянемо деякі із знайдених помилок.

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

Попередження N1

Попередження аналізатора: V3005 '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, яка завжди правда, а значить виконається рядок 3. Якщо ж умова 1 виявиться хибним, то ми перейдемо на рядок, позначений цифрою 4, умова в якій теж завжди правдаОтже, виконається рядок 5. Таким чином, умови, позначені коментарем 0, ніколи не будуть виконані, що може бути не зовсім тією логікою роботи, на яку розраховував програміст.

Попередження N5

Попередження аналізатора: V3123 [CWE-783] Perhaps the '?:' роботодавець працює в різних випадках, тому що він був виявлений. Її priority is lower than priority інших операторів в його 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, яке завжди буде правда. Часто таке попередження вказує на логічну помилку в коді, а в інших випадках просто надмірну перевірку.

Попередження N7

Попередження аналізатора: V3001 Є identical sub-expressions 'commandArguments.contains("-apikey")' до лівого і правого '||' 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 помилки мають великий шанс рано чи пізно з'явитися в будь-якому проекті з великою кількістю вихідного коду, і один із найкращих засобів боротьби з ними – статичний аналіз.

PS І як завжди, ця помилка тяжіє з'явитися наприкінці багаторядкової умови:). Див публікацію "Ефект останнього рядка".

Попередження N8

Попередження аналізатора: V3095 [CWE-476] 'installedpackage' об'єкт був використаний перед тим, що він був здійснений до 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 використовується, а потім перевіряється на нулю. Ця діагностика говорить нам про одну з двох проблем у програмі: або installedPackage ніколи не дорівнює нулю, Що сумнівно, і тоді перевірка надмірна, або потенційно можемо отримати серйозну помилку в коді – спробу доступу за нульовим посиланням.

Висновок

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

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

PS

Перед публікацією ми надіслали статтю розробникам Сhocolatey, і вони її добре прийняли. Нічого критичного нами не було знайдено, але їм, наприклад, сподобалася знайдена нами помилка, пов'язана з ключем «api-key».

Якщо хочете поділитися цією статтею з англомовною аудиторією, прошу використати посилання на переклад: Vladislav Stolyarov. PVS-Studio Is Now in Chocolatey: Checking Chocolatey під Azure DevOps.

Джерело: habr.com