PVS-Studio teraz w Chocolatey: sprawdzanie Chocolatey z poziomu Azure DevOps

Kontynuujemy ułatwiać korzystanie z PVS-Studio. Nasz analizator jest teraz dostępny w Chocolatey, menedżerze pakietów dla systemu Windows. Uważamy, że to uprości wdrażanie PVS-Studio, szczególnie w chmurze. Aby nie szukać daleko, sprawdźmy kod źródłowy tego samego Chocolatey. Jako system CI użyjemy Azure DevOps.

Oto lista naszych innych artykułów na temat integracji z systemami chmurowymi:

• PVS-Studio w chmurze: Azure DevOps
• PVS-Studio w chmurze: Travis CI
• PVS-Studio w chmurze: CircleCI
• PVS-Studio w chmurze: GitLab CI/CD

Zalecam zwrócenie uwagi na pierwszy artykuł dotyczący integracji z Azure DevOps, ponieważ w tym przypadku pewne kwestie zostały pominięte, aby uniknąć powtórzeń.

A więc, bohaterowie tego artykułu:

PVS-Studio — narzędzie do statycznej analizy kodu, przeznaczone do wykrywania błędów i potencjalnych luk w programach napisanych w językach C, C++, C# i Java. Działa w 64-bitowych systemach na Windows, Linux i macOS oraz może analizować kod przeznaczony dla platform 32-bitowych, 64-bitowych i wbudowanych ARM. Jeśli po raz pierwszy spróbujesz statycznej analizy kodu do sprawdzenia swoich projektów, polecam zapoznać się z artykułu o tym, jak szybko obejrzeć najbardziej interesujące ostrzeżenia PVS-Studio i ocenić możliwości tego narzędzia.

Azure DevOps — zestaw usług chmurowych, wspólnie obejmujących cały proces tworzenia oprogramowania. W skład tej platformy wchodzą takie narzędzia jak Azure Pipelines, Azure Boards, Azure Artifacts, Azure Repos, Azure Test Plans, które przyspieszają proces tworzenia oprogramowania i poprawiają jego jakość.

Chocolatey – menedżer pakietów dla Windows z otwartym kodem źródłowym. Celem projektu jest zautomatyzowanie całego cyklu życia oprogramowania, od instalacji po aktualizację i usuwanie w systemach operacyjnych Windows.

O korzystaniu z Chocolatey

Możesz zobaczyć, jak zainstalować sam menedżer pakietów, korzystając z tej linkiem. Pełna dokumentacja dotycząca instalacji analizatora jest dostępna pod linkiem w sekcji „Instalacja przy użyciu menedżera pakietów Chocolatey”. W skrócie powtórzę niektóre punkty stamtąd.

Polecenie do instalacji najnowszej wersji analizatora:

choco install pvs-studio

Polecenie instalacji konkretnej wersji pakietu PVS-Studio:

choco install pvs-studio --version=7.05.35617.2075

Domyślnie instalowany jest tylko rdzeń analizatora — komponent Core. Wszystkie pozostałe flagi (Standalone, JavaCore, IDEA, MSVS2010, MSVS2012, MSVS2013, MSVS2015, MSVS2017, MSVS2019) można przekazać za pomocą —package-parameters.

Przykład polecenia, które zainstaluje analizator z wtyczką dla Visual Studio 2019:

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

Teraz przyjrzymy się przykładowi wygodnego użycia analizatora w Azure DevOps.

Konfiguracja

Przypominam, że o takich kwestiach, jak rejestracja konta, tworzenie Build Pipeline oraz synchronizacja konta z projektem w repozytorium na GitHubie, jest osobny artykuł. Nasza konfiguracja zacznie się od napisania pliku konfiguracyjnego.

Na początek skonfigurujemy wyzwalacz uruchamiania, podając, że uruchamiamy tylko dla zmian w master gałęzi:

trigger:
- master

Następnie musimy wybrać maszynę wirtualną. Na ten moment będzie to agent hostowany przez Microsoft z systemem Windows Server 2019 i Visual Studio 2019:

pool:
  vmImage: 'windows-latest'

Przejdźmy do treści pliku konfiguracyjnego (bloku steps). Pomimo tego, że na maszynie wirtualnej nie można zainstalować dowolnego oprogramowania, nie dodawałem kontenera Docker. Możemy dodać Chocolatey jako rozszerzenie dla Azure DevOps. W tym celu przejdźmy pod linkiem. Klikamy Get it free. Następnie, jeśli jesteś już zalogowany, po prostu wybieramy swoje konto, a jeśli nie, to wykonujemy wszystko to samo po zalogowaniu.

Tu trzeba wybrać, gdzie dodamy rozszerzenie, i kliknąć przycisk Zainstaluj.

Po pomyślnej instalacji klikamy Proceed to organization:

Teraz możemy zobaczyć szablon zadania Chocolatey w oknie tasks podczas edytowania pliku konfiguracyjnego azure-pipelines.yml:

Klikamy na Chocolatey i widzimy listę pól:

Tutaj musimy wybrać install w polu z poleceniami. W Nuspec File Name podajemy nazwę potrzebnego pakietu – pvs-studio. Jeśli nie podamy wersji, zainstaluje się najnowsza, co nam całkowicie odpowiada. Klikamy przycisk add i widzimy utworzone zadanie w pliku konfiguracyjnym.

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

Następnie przejdziemy do głównej części naszego pliku:

- task: CmdLine@2
  inputs:
    script: 

Teraz musimy utworzyć plik z licencją analizatora. Tutaj PVSNAME i PVSKEY – to nazwy zmiennych, których wartości podajemy w ustawieniach. Będą przechowywać login i klucz licencyjny PVS-Studio. Aby ustawić ich wartości, otworzymy menu Variables->New variable. Utworzymy zmienne PVSNAME dla logina i PVSKEY do klucza analizatora. Nie zapomnij zaznaczyć pola Zachowaj tę wartość w tajemnicy do PVSKEY. Kod polecenia:

call "C:Program Files (x86)PVS-StudioPVS-Studio_Cmd.exe" credentials 
–u $(PVSNAME) –n $(PVSKEY)

Zbudujemy projekt za pomocą pliku bat znajdującego się w repozytorium:

call build.bat

Utworzymy folder, w którym będą przechowywane pliki z wynikami pracy analizatora:

call mkdir PVSTestResults

Rozpoczniemy analizę projektu:

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

Skonwertujemy nasz raport do formatu html za pomocą narzędzia PlogConverter:

call "C:Program Files (x86)PVS-StudioPlogConverter.exe" 
–t html –o PVSTestResults .PVSTestResultsChoco.plog

Teraz musimy stworzyć zadanie, aby można było wyeksportować raport.

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

Pełny plik konfiguracyjny wygląda tak:

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()

Naciśnijmy Zapisz->Zapisz->Uruchom do uruchomienia zadania. Wyeksportujemy raport, przechodząc do zakładki zadań.

Projekt Chocolatey zawiera jedynie 37615 linii kodu C#. Przyjrzyjmy się niektórym z znalezionych błędów.

Wyniki kontroli

Ostrzeżenie N1

Ostrzeżenie analizatora: V3005 Zmienna ‘Provider’ jest przypisywana do samej siebie. CrytpoHashProviderSpecs.cs 38

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

Analizator wykrył przydzielenie zmiennej samej sobie, co nie ma sensu. Prawdopodobnie w miejscu jednej z tych zmiennych powinna być inna. Ewentualnie to literówka i zbędne przypisanie można po prostu usunąć.

Ostrzeżenie N2

Ostrzeżenie analizatora: V3093 [CWE-480] Operator ‘&’ ocenia oba operandy. Może lepszym rozwiązaniem byłby operator krótkiego działania ‘&&’. 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;
  }
}

Różnica między operatorem & a operatorem && polega na tym, że jeśli lewa część wyrażenia — false, i tak czy inaczej będzie obliczana prawa strona, co w tym przypadku oznacza dodatkowe wywołania metody system.directory_exists.

W analizowanym fragmencie to drobny błąd. Tak, ten warunek można zoptymalizować, zastępując operator & operatorem &&, ale z praktycznego punktu widzenia nie ma to na nic wpływu. Jednak w innych przypadkach mylenie & i && może prowadzić do poważnych problemów, gdy prawa strona wyrażenia będzie pracować z niepoprawnymi/niedopuszczalnymi wartościami. Na przykład, w naszej kolekcji błędów, wykrytych podczas diagnostyki V3093, jest taki przypadek:

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

Nawet jeśli indeks k jest niepoprawny, będzie używany do dostępu do elementu tablicy. W wyniku tego zgłoszone zostanie wyjątek IndexOutOfRangeException.

Ostrzeżenia N3, N4

Ostrzeżenie analizatora: V3022 [CWE-571] Wyrażenie ‘shortPrompt’ jest zawsze prawdziwe. InteractivePrompt.cs 101
Ostrzeżenie analizatora: V3022 [CWE-571] Wyrażenie ‘shortPrompt’ jest zawsze prawdziwe. 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
    ....
  }
  ....
}

W tym przypadku mamy do czynienia z dziwną logiką działania operatora warunkowego. Przyjrzyjmy się dokładniej: jeśli spełnione zostanie warunek oznaczony przeze mnie cyfrą 1, to przejdziemy do warunku 2, który zawsze true, a więc wykona się linijka 3. Jeśli jednak warunek 1 okaże się fałszywy, to przejdziemy do linijki oznaczonej cyfrą 4, gdzie warunek również zawsze true, a więc wykona się linijka 5. W ten sposób warunki oznaczone komentarzem 0 nigdy nie będą spełnione, co może nie być do końca tym, na co liczył programista.

Ostrzeżenie N5

Ostrzeżenie analizatora: V3123 [CWE-783] Być może operator ‘?:’ działa w inny sposób, niż się tego spodziewano. Jego priorytet jest niższy niż priorytet innych operatorów w jego warunku. 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 = 0 && j != 0 ? description [j++ - 1] == '{' : false);
    ....
    return maxIndex == 1 ? "VALUE" : "VALUE" + (index + 1);
  }
}

Diagnostyka zadziałała na linijce:

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

Ponieważ zmienna j w kilku liniach wyżej jest inicjowana jako zero, operator warunkowy zwróci wartość false. Z powodu tego warunku, ciało pętli zostanie wykonane tylko raz. Wydaje mi się, że ten fragment kodu nie działa tak, jak zamierzał programista.

Ostrzeżenie N6

Ostrzeżenie analizatora: V3022 [CWE-571] Wyrażenie ‘installedPackageVersions.Count != 1’ jest zawsze prawdziwe. NugetService.cs 1405

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

Tutaj jest dziwne zagnieżdżone warunkowe: installedPackageVersions.Count != 1, które zawsze będzie true. Często takie ostrzeżenie wskazuje na błąd logiczny w kodzie, a w pozostałych przypadkach po prostu na zbędną weryfikację.

Ostrzeżenie N7

Ostrzeżenie analizatora: V3001 Istnieją identyczne podwyrażenia ‘commandArguments.contains("-apikey")’ po lewej i prawej stronie operatora ‘||’. 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");
}

Programista, który napisał ten fragment kodu, skopiował dwie ostatnie linijki i zapomniał je edytować. Z tego powodu użytkownicy Chocolatey utracili możliwość zastosowania parametru apikey jeszcze kilkoma sposobami. Podobnie jak parametry powyżej, mogę zaproponować takie opcje:

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

Błędy copy-paste mają duże szanse na pojawienie się prędzej czy później w każdym projekcie z dużą ilością kodu źródłowego, a jednym z najlepszych narzędzi do walki z nimi jest analiza statyczna.

P.S. I jak zawsze, ten błąd ma tendencję do pojawiania się na końcu wieloliniowego warunku :). Zobacz publikację "Efekt ostatniej linii".

Ostrzeżenie N8

Ostrzeżenie analizatora: V3095 [CWE-476] Obiekt ‘installedPackage’ był używany przed jego weryfikacją pod kątem null. Sprawdź linie: 910, 917. NugetService.cs 910

public virtual ConcurrentDictionary 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)
  {
    ....
  }
  ....
}

Klasyczny błąd: najpierw używa się obiektu installedPackage a potem sprawdza się go pod kątem null. Ta diagnoza wskazuje na jeden z dwóch problemów w programie: albo installedPackage nigdy nie jest równy null, co jest wątpliwe, a wtedy sprawdzenie jest zbędne, albo możemy potencjalnie napotkać poważny błąd w kodzie – próbę dostępu do pustego wskaźnika.

Podsumowanie

Oto wykonaliśmy jeszcze jeden mały krok – teraz korzystanie z PVS-Studio stało się jeszcze prostsze i wygodniejsze. Chciałbym też powiedzieć, że Chocolatey to dobry menedżer pakietów z niewielką liczbą błędów w kodzie, które mogłyby być jeszcze mniejsze przy użyciu PVS-Studio.

Zapraszamy pobierz i wypróbować PVS-Studio. Regularne korzystanie ze statycznego analizatora zwiększy jakość i niezawodność kodu opracowanego przez twój zespół i pomoże zapobiec wielu lukom zero-day.

P.S.

Przed publikacją wysłaliśmy artykuł do deweloperów Chocolatey, i dobrze go przyjęli. Nie znaleźliśmy nic krytycznego, ale im na przykład spodobał się błąd, który znaleźliśmy związany z kluczem „api-key”.

Jeśli chcesz podzielić się tym artykułem z anglojęzyczną publicznością, proszę użyj linku do tłumaczenia: Vladislav Stolyarov. PVS-Studio jest teraz w Chocolatey: Sprawdzanie Chocolatey w Azure DevOps.

Źródło: habr.com