O PVS-Studio vai para as nuvens: Azure DevOps

Este é o segundo artigo sobre o uso do analisador estático PVS-Studio em sistemas de CI na nuvem e, desta vez, consideraremos a plataforma Azure DevOps - uma solução de CI \ CD na nuvem da Microsoft. Como um projeto analisado desta vez, considere o ShareX.

Vamos precisar de três componentes. O primeiro é o analisador estático PVS-Studio. O segundo é o Azure DevOps, com o qual integraremos o analisador. O terceiro é um projeto que verificaremos para demonstrar os recursos do PVS-Studio ao trabalhar na nuvem. Então, vamos começar.

O PVS-Studio é um analisador de código estático para procurar erros e defeitos de segurança. Executa análise de código em C, C ++, C # e Java.

DevOps do Azure . A plataforma Azure DevOps inclui ferramentas como o Pipeline do Azure, a Placa do Azure, os Artefatos do Azure e outras para acelerar o processo de criação de software e melhorar sua qualidade.

O ShareX é um aplicativo gratuito que permite capturar e gravar qualquer parte da tela. O projeto está escrito em C # e é ótimo para demonstrar como executar o analisador estático. O código fonte do projeto está disponível no GitHub .

A saída do comando cloc para o projeto ShareX:
Em outras palavras, o projeto é pequeno, mas suficiente para demonstrar o trabalho do PVS-Studio em conjunto com uma plataforma em nuvem.

Vamos configurar

Para começar no Azure DevOps, clique no link e clique no botão "Iniciar gratuitamente com o GitHub".

Conceder ao aplicativo Microsoft acesso aos dados da conta do GitHub.

Para concluir o registro terá que criar uma conta da Microsoft.

Após o registro, crie um projeto:

Em seguida, precisamos ir para a seção "Pipelines" - "Builds" e criar um novo pipeline de Build

Para a pergunta onde nosso código está localizado, responderemos - GitHub.

Autorizamos o aplicativo Pipelines do Azure e selecionamos o repositório com o projeto para o qual configuraremos o lançamento do analisador estático

Na janela de seleção de modelos, selecione "Starter pipeline".

Podemos executar a análise estática do código do projeto de duas maneiras: usando agentes hospedados ou auto-hospedados pela Microsoft.

Na primeira versão, usaremos agentes hospedados pela Microsoft. Esses agentes são máquinas virtuais comuns que iniciam quando iniciamos nosso pipeline e são excluídas após o término da tarefa. O uso desses agentes permite que você não perca tempo dando suporte e atualizando-os, mas impõe algumas restrições, por exemplo, a impossibilidade de instalar software adicional usado para construir o projeto.

Substitua nossa configuração padrão pelo seguinte para usar agentes hospedados pela Microsoft:

#    #      master- trigger: - master #         # ,   Docker-, #      Windows Server 1803 pool: vmImage: 'win1803' container: microsoft/dotnet-framework:4.7.2-sdk-windowsservercore-1803 steps: #    - task: PowerShell@2 inputs: targetType: 'inline' script: 'Invoke-WebRequest -Uri https://files.viva64.com/PVS-Studio_setup.exe -OutFile PVS-Studio_setup.exe' - task: CmdLine@2 inputs: workingDirectory: $(System.DefaultWorkingDirectory) script: | #      nuget restore .\ShareX.sln #  ,        md .\PVSTestResults #   PVS-Studio_setup.exe /VERYSILENT /SUPPRESSMSGBOXES /NORESTART /COMPONENTS=Core #        "C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe" credentials -u $(PVS_USERNAME) -n $(PVS_KEY) #        html. "C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe" -t .\ShareX.sln -o .\PVSTestResults\ShareX.plog "C:\Program Files (x86)\PVS-Studio\PlogConverter.exe" -t html -o .\PVSTestResults\ .\PVSTestResults\ShareX.plog #    - task: PublishBuildArtifacts@1 inputs: pathToPublish: PVSTestResults artifactName: PVSTestResults 

Nota: de acordo com a documentação , o contêiner usado deve ser armazenado em cache na imagem da máquina virtual, mas no momento da redação deste artigo não funciona e o contêiner é baixado sempre que a tarefa é iniciada, o que afeta negativamente o tempo de execução.

Salve o pipeline e crie as variáveis ​​que serão usadas para criar o arquivo de licença. Para fazer isso, abra a janela de edição do pipeline e, no canto superior direito, clique no botão "Variáveis".

Adicione duas variáveis ​​- PVS_USERNAME e PVS_KEY , contendo o nome de usuário e a chave de licença, respectivamente. Ao criar a variável PVS_KEY , não se esqueça de verificar o item "Manter este valor em segredo" para criptografar o valor da variável com uma chave RSA de 2048 bits, além de suprimir a saída do valor da variável no log de execução da tarefa.

Nós salvamos as variáveis ​​e iniciamos o pipeline com o botão "Executar".

A segunda opção para executar a análise é usar um agente auto-hospedado. Agentes auto-hospedados são agentes que configuramos e gerenciamos por nós mesmos. Esses agentes oferecem mais oportunidades para a instalação de software, o que é necessário para a montagem e teste de nosso produto de software.

Antes de usar esses agentes, eles devem ser configurados de acordo com as instruções e um analisador estático deve ser instalado e configurado .

Para iniciar a tarefa em um agente auto-hospedado, substituímos a configuração padrão proposta pelo seguinte:

 #    #    master- trigger: - master #    self-hosted    'MyPool' pool: 'MyPool' steps: - task: CmdLine@2 inputs: workingDirectory: $(System.DefaultWorkingDirectory) script: | #      nuget restore .\ShareX.sln #  ,        md .\PVSTestResults #        html. "C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe" -t .\ShareX.sln -o .\PVSTestResults\ShareX.plog "C:\Program Files (x86)\PVS-Studio\PlogConverter.exe" -t html -o .\PVSTestResults\ .\PVSTestResults\ShareX.plog #    - task: PublishBuildArtifacts@1 inputs: pathToPublish: PVSTestResults artifactName: PVSTestResults 

Depois de concluir a tarefa, o arquivo com os relatórios do analisador pode ser baixado na guia Resumo ou podemos usar a extensão Enviar email , que permite configurar o envio de emails ou procurar uma ferramenta mais conveniente para nós no Marketplace .

Sobre os resultados da análise

Agora, vejamos alguns dos erros encontrados no projeto verificado - ShareX.

Verificações redundantes

Para aquecer, vamos começar com falhas simples no código, nomeadamente com verificações redundantes:

 private void PbThumbnail_MouseMove(object sender, MouseEventArgs e) { .... IDataObject dataObject = new DataObject(DataFormats.FileDrop, new string[] { Task.Info.FilePath }); if (dataObject != null) { Program.MainForm.AllowDrop = false; dragBoxFromMouseDown = Rectangle.Empty; pbThumbnail.DoDragDrop(dataObject, DragDropEffects.Copy | DragDropEffects.Move); Program.MainForm.AllowDrop = true; } .... } 

Aviso do PVS-Studio : V3022 [CWE-571] A expressão 'dataObject! = Null' sempre é verdadeira. TaskThumbnailPanel.cs 415

Preste atenção na verificação da variável dataObject para null . Para que ela está aqui? dataObject simplesmente não pode ser nulo neste caso, pois é inicializado com uma referência ao objeto criado. Como resultado, temos uma verificação redundante. Isso é crítico? Não. Parece conciso? Não. Essa verificação é claramente melhor removida para não bagunçar o código.

Vamos dar uma olhada em outro trecho de código, para o qual você pode fazer comentários semelhantes:

 private static Image GetDIBImage(MemoryStream ms) { .... try { .... return new Bitmap(bmp); .... } finally { if (gcHandle != IntPtr.Zero) { GCHandle.FromIntPtr(gcHandle).Free(); } } .... } private static Image GetImageAlternative() { .... using (MemoryStream ms = dataObject.GetData(format) as MemoryStream) { if (ms != null) { try { Image img = GetDIBImage(ms); if (img != null) { return img; } } catch (Exception e) { DebugHelper.WriteException(e); } } } .... } 

Aviso do PVS-Studio : V3022 [CWE-571] A expressão 'img! = Null' sempre é verdadeira. ClipboardHelpers.cs 289

O método GetImageAlternative novamente verifica se a variável img não é nula imediatamente após a criação de uma nova instância da classe Bitmap . A diferença do exemplo anterior aqui é que, para inicializar a variável img , não usamos o construtor, mas o método GetDIBImage . O autor do código pressupõe que uma exceção possa ocorrer nesse método, mas declara apenas try e finalmente bloqueia, omitindo catch . Portanto, se ocorrer uma exceção, o método de chamada - GetImageAlternative - não receberá uma referência a um objeto do tipo Bitmap, mas será forçado a manipular a exceção em seu próprio bloco de captura . Nesse caso, a variável img não será inicializada e o encadeamento de execução nem chegará à verificação img! = Null , mas cairá imediatamente no bloco catch . Portanto, o analisador indicou validação redundante.

Considere o seguinte exemplo de aviso com o código V3022 :

 private void btnCopyLink_Click(object sender, EventArgs e) { .... if (lvClipboardFormats.SelectedItems.Count == 0) { url = lvClipboardFormats.Items[0].SubItems[1].Text; } else if (lvClipboardFormats.SelectedItems.Count > 0) { url = lvClipboardFormats.SelectedItems[0].SubItems[1].Text; } .... } 

Aviso do PVS-Studio : V3022 [CWE-571] A expressão 'lvClipboardFormats.SelectedItems.Count> 0' sempre é verdadeira. AfterUploadForm.cs 155

Vamos olhar para a segunda expressão condicional. Lá, verificamos o valor da propriedade Count somente leitura. Esta propriedade mostra o número de elementos em uma instância da coleção SelectedItems . A condição será satisfeita apenas se a propriedade Count for maior que zero. Tudo ficaria bem, mas é apenas na declaração externa if que o Count já está marcado. Uma instância da coleção SelectedItems não pode ter o número de elementos menor que zero; portanto, Count assume um valor igual a zero ou maior que zero. Como já executamos uma verificação na primeira instrução if de que Count é zero e acabou sendo falso, não faz sentido escrever outra verificação no ramo else de que Count é maior que zero.

O exemplo final do número de erro V3022 é o seguinte fragmento de código:

 private void DrawCursorGraphics(Graphics g) { .... int cursorOffsetX = 10, cursorOffsetY = 10, itemGap = 10, itemCount = 0; Size totalSize = Size.Empty; int magnifierPosition = 0; Bitmap magnifier = null; if (Options.ShowMagnifier) { if (itemCount > 0) totalSize.Height += itemGap; .... } .... } 

PVS-Studio Warning : A expressão V3022 'itemCount> 0' é sempre falsa. RegionCaptureForm.cs 1100.

O analisador notou que a condição itemCount> 0 sempre será falsa, pois é executada uma declaração um pouco mais alta e a variável itemCount é definida como zero ao mesmo tempo. Até a própria condição, essa variável não é usada em nenhum lugar e não muda; portanto, o analisador fez a conclusão correta sobre a expressão condicional, cujo valor é sempre falso.

Bem, agora vamos olhar para algo realmente interessante.

A melhor maneira de entender o erro é visualizando o erro.

Parece-nos que um erro bastante interessante foi encontrado neste local:

 public static void Pixelate(Bitmap bmp, int pixelSize) { .... float r = 0, g = 0, b = 0, a = 0; float weightedCount = 0; for (int y2 = y; y2 < yLimit; y2++) { for (int x2 = x; x2 < xLimit; x2++) { ColorBgra color = unsafeBitmap.GetPixel(x2, y2); float pixelWeight = color.Alpha / 255; r += color.Red * pixelWeight; g += color.Green * pixelWeight; b += color.Blue * pixelWeight; a += color.Alpha * pixelWeight; weightedCount += pixelWeight; } } .... ColorBgra averageColor = new ColorBgra((byte)(b / weightedCount), (byte)(g / weightedCount), (byte)(r / weightedCount), (byte)(a / pixelCount)); .... } 

Não quero revelar imediatamente todos os cartões e mostrar o que nosso analisador encontrou aqui. Adiamos esse momento por um breve período.

Pelo nome do método, é fácil adivinhar o que ele faz - você envia uma imagem ou um fragmento da imagem como entrada e ele executa sua pixelização. O código do método é bastante longo, portanto, não o forneceremos aqui na íntegra, mas simplesmente tente explicar seu algoritmo e que tipo de bug o PVS-Studio encontrou aqui.

Este método aceita dois parâmetros como entrada: um objeto do tipo Bitmap e um valor do tipo int , que indica o tamanho dos pixels. O algoritmo de operação é bastante simples:

1) Dividimos o fragmento da imagem recebida na entrada em quadrados com um lado igual ao tamanho da pixelização. Por exemplo, se tivermos um tamanho de pixelização igual a 15, obteremos um quadrado contendo 15x15 = 225 pixels.

2) Em seguida, contornamos cada pixel neste quadrado e acumulamos os valores dos campos Vermelho , Verde , Azul e Alfa em variáveis ​​intermediárias e multiplicamos anteriormente o valor da cor correspondente e o valor do canal alfa pela variável pixelWeight , obtida dividindo o valor Alfa por 255 (a variável Alfa possui tipo byte ). Além disso, ao atravessar pixels, somamos os valores registrados em pixelWeight em uma variável chamada weightedCount .

O trecho de código que executa as etapas acima é o seguinte:

 ColorBgra color = unsafeBitmap.GetPixel(x2, y2); float pixelWeight = color.Alpha / 255; r += color.Red * pixelWeight; g += color.Green * pixelWeight; b += color.Blue * pixelWeight; a += color.Alpha * pixelWeight; weightedCount += pixelWeight; 

A propósito, observe que, se o valor da variável Alpha for zero, o pixelWeight não adicionará nenhum valor à variável weightedCount desse pixel. Nós precisaremos disso no futuro.

3) Depois de contornar todos os pixels no quadrado atual, podemos criar a cor "média" geral para esse quadrado. O código que executa essas ações é o seguinte:

 ColorBgra averageColor = new ColorBgra((byte)(b / weightedCount), (byte)(g / weightedCount), (byte)(r / weightedCount), (byte)(a / pixelCount)); 

4) Agora que temos a cor final e a escrevemos na variável averageColor , podemos novamente contornar cada pixel no quadrado e atribuir um valor a partir de averageColor .

5) Retornamos à etapa 2, desde que ainda haja quadrados brutos.

Mais uma vez, observamos que a variável weightedCount não é igual ao número de todos os pixels ao quadrado. Por exemplo, se um pixel absolutamente transparente ocorrer na imagem (o valor é zero no canal alfa), a variável pixelWeight será zero para esse pixel ( 0/255 = 0); portanto, esse pixel não fará nenhuma contribuição para a formação do valor da variável weightedCount . Isso é lógico - não faz sentido levar em consideração as cores de um pixel absolutamente transparente.

Tudo parece bastante razoável - a pixelização deve funcionar corretamente. E realmente funciona bem. Isso não é apenas para imagens png que possuem pixels com valores no canal alfa menores que 255 e diferentes de zero. Preste atenção à imagem pixelizada abaixo:

Você viu pixelização? E nós não somos. Bem, agora vamos revelar essa pequena intriga e explicar onde exatamente o bug está oculto nesse método. O erro entrou na linha para calcular o valor da variável pixelWeight :

 float pixelWeight = color.Alpha / 255; 

O fato é que o autor do código, declarando a variável pixelWeight como float , implicava que, ao dividir o campo Alpha por 255, além de zero e um, números fracionários deveriam ser obtidos. É aqui que está o problema, já que a variável Alpha é do tipo byte e, quando a dividimos por 255, obtemos um valor inteiro e, somente então, ela é lançada implicitamente para flutuar , portanto, a parte fracionária é perdida.

A incapacidade de pixelizar imagens PNG com algum grau de transparência é fácil de explicar. Como os valores do canal alfa desses pixels estão no intervalo 0 <Alpha <255, ao dividir a variável Alpha por 255, sempre obteremos 0. Portanto, os valores das variáveis ​​pixelWeight , r , g , b , a , weightedCount também são sempre será zero. Como resultado, nossa cor média averageColor estará com valores zero em todos os canais: vermelho - 0, azul - 0, verde - 0, alpha - 0. Preenchendo o quadrado com essa cor, não alteramos a cor original dos pixels, já que averageColor é absolutamente transparente . Para corrigir esse erro, você só precisa converter explicitamente o campo Alfa no tipo flutuante . A linha de código corrigida pode ser assim:

 float pixelWeight = (float)color.Alpha / 255; 

E é hora de citar a mensagem que o PVS-Studio deu ao código incorreto:

PVS-Studio Warning : V3041 [CWE-682] A expressão foi convertida implicitamente do tipo 'int' para o tipo 'float'. Considere utilizar uma conversão de tipo explícita para evitar a perda de uma parte fracionária. Um exemplo: double A = (double) (X) / Y; ImageHelpers.cs 1119.

E, para comparação, fornecemos uma captura de tela de uma imagem verdadeiramente pixelizada obtida em uma versão fixa do aplicativo:

Potencial NullReferenceException

 public static bool AddMetadata(Image img, int id, string text) { .... pi.Value = bytesText; if (pi != null) { img.SetPropertyItem(pi); return true; } .... } 

Aviso do PVS-Studio: V3095 [CWE-476] O objeto 'pi' foi usado antes de ser verificado como nulo. Verifique as linhas: 801, 803. ImageHelpers.cs 801

Esse fragmento de código mostra que seu autor esperava que a variável pi fosse nula , e é por isso que a verificação pi! = Null é realizada antes de chamar o método SetPropertyItem . É estranho que antes dessa verificação, uma matriz de bytes seja atribuída à propriedade pi.Value , porque se pi for nulo , uma exceção do tipo NullReferenceException será lançada.

Uma situação semelhante foi vista em outro lugar:

 private static void Task_TaskCompleted(WorkerTask task) { .... task.KeepImage = false; if (task != null) { if (task.RequestSettingUpdate) { Program.MainForm.UpdateCheckStates(); } .... } .... } 

Aviso do PVS-Studio: V3095 [CWE-476] O objeto 'task' foi usado antes de ser verificado como nulo. Verifique as linhas: 268, 270. TaskManager.cs 268

O PVS-Studio encontrou outro erro semelhante. O significado ainda é o mesmo; portanto, não há grande necessidade de fornecer um fragmento de código; nos restringimos à mensagem do analisador.

Aviso do PVS-Studio: V3095 [CWE-476] O objeto 'Config.PhotobucketAccountInfo' foi usado antes de ser verificado como nulo. Verifique as linhas: 216, 219. UploadersConfigForm.cs 216

O mesmo valor de retorno

Um trecho de código suspeito foi descoberto no método EvalWindows da classe WindowsList , que retorna true em qualquer circunstância:

 public class WindowsList { public List<IntPtr> IgnoreWindows { get; set; } .... public WindowsList() { IgnoreWindows = new List<IntPtr>(); } public WindowsList(IntPtr ignoreWindow) : this() { IgnoreWindows.Add(ignoreWindow); } .... private bool EvalWindows(IntPtr hWnd, IntPtr lParam) { if (IgnoreWindows.Any(window => hWnd == window)) { return true; // <= } windows.Add(new WindowInfo(hWnd)); return true; // <= } } 

PVS-Studio Warning: V3009 É estranho que esse método sempre retorne um e o mesmo valor de 'true'. WindowsList.cs 82

Parece lógico que, se um ponteiro com o mesmo valor que hWnd fosse encontrado na lista com o nome IgnoreWindows , o método retornaria false .

A lista IgnoreWindows pode ser preenchida chamando o construtor WindowsList (IntPtr ignoreWindow) ou diretamente através do acesso à propriedade, pois ela é pública. De uma forma ou de outra, de acordo com o Visual Studio, no momento, no código, essa lista não é preenchida de nenhuma maneira. Este é outro lugar estranho desse método.

Chamada insegura para manipuladores de eventos

 protected void OnNewsLoaded() { if (NewsLoaded != null) { NewsLoaded(this, EventArgs.Empty); } } 

Aviso do PVS-Studio: V3083 [CWE-367] Chamada não segura do evento 'NewsLoaded', NullReferenceException é possível. Considere atribuir um evento a uma variável local antes de invocá-lo. NewsListControl.cs 111

Nesse caso, a seguinte situação desagradável pode ocorrer: após verificar a variável NewsLoaded em busca de desigualdade nula , o método que processa o evento pode ser cancelado, por exemplo, em outro thread, e quando entrarmos no corpo da instrução if condicional, a variável NewsLoaded já estará é igual a nulo . Tentar chamar assinantes em um evento NewsLoaded que seja nulo lançará uma NullReferenceException . É muito mais seguro usar o operador condicional nulo e reescrever o código acima da seguinte maneira:

 protected void OnNewsLoaded() { NewsLoaded?.Invoke(this, EventArgs.Empty); } 

O analisador indicou mais 68 lugares semelhantes. Não os descreveremos aqui - o padrão da chamada de evento neles é semelhante.

Retornar nulo de ToString

Há pouco tempo, no artigo interessante de um colega , descobri que a Microsoft não recomenda retornar nulo de um método ToString substituído. O PVS-Studio está ciente disso:

 public override string ToString() { lock (loggerLock) { if (sbMessages != null && sbMessages.Length > 0) { return sbMessages.ToString(); } return null; } } 

Aviso do PVS-Studio: V3108 Não é recomendável retornar 'null' do método 'ToSting ()'. Logger.cs 167

Por que apropriado se não estiver usando?

 public SeafileCheckAccInfoResponse GetAccountInfo() { string url = URLHelpers.FixPrefix(APIURL); url = URLHelpers.CombineURL(APIURL, "account/info/?format=json"); .... } 

PVS-Studio Warning: V3008 A variável 'url' recebe valores duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 197, 196. Seafile.cs 197

Como você pode ver no exemplo, ao declarar a variável url , é atribuído algum valor retornado do método FixPrefix . Na próxima linha, "moeremos" o valor resultante, mesmo sem usá-lo em nenhum lugar. Temos algo parecido com "código morto" - ele faz o trabalho, não afeta o resultado final. Esse erro provavelmente é o resultado de copiar e colar, pois esses fragmentos de código são encontrados em mais 9 métodos.
Por exemplo, fornecemos dois métodos com uma primeira linha semelhante:

 public bool CheckAuthToken() { string url = URLHelpers.FixPrefix(APIURL); url = URLHelpers.CombineURL(APIURL, "auth/ping/?format=json"); .... } .... public bool CheckAPIURL() { string url = URLHelpers.FixPrefix(APIURL); url = URLHelpers.CombineURL(APIURL, "ping/?format=json"); .... } 

Total

Como podemos ver, a complexidade da configuração da verificação automática pelo analisador não depende do sistema de IC selecionado - em apenas 15 minutos e com apenas alguns cliques do mouse, configuramos a verificação do código do nosso projeto com um analisador estático.

Concluindo, sugerimos que você baixe e experimente o analisador em seus projetos.

Se você deseja compartilhar este artigo com um público que fala inglês, use o link para a tradução: Oleg Andreev, Ilya Gainulin. PVS-Studio nas nuvens: Azure DevOps .