Ceci est un deuxième article, qui se concentre sur l'utilisation de l'analyseur PVS-Studio dans les systèmes CI cloud. Cette fois, nous considérerons la plateforme Azure DevOps - une solution cloud CI \ CD de Microsoft. Nous analyserons le projet ShareX.
Nous aurons besoin de trois composants. Le premier est l'analyseur PVS-Studio. Le second est Azure DevOps, avec lequel nous intégrerons l'analyseur. Le troisième est le projet que nous allons vérifier afin de démontrer les capacités de PVS-Studio lorsque vous travaillez dans un cloud. Alors allons-y.
PVS-Studio est un analyseur de code statique pour rechercher les erreurs et les défauts de sécurité. L'outil prend en charge l'analyse du code C, C ++ et C #.
Azure DevOps . La plateforme Azure DevOps comprend des outils tels que Azure Pipeline, Azure Board, Azure Artifacts et d'autres qui accélèrent le processus de création de logiciels et améliorent sa qualité.
ShareX est une application gratuite qui vous permet de capturer et d'enregistrer n'importe quelle partie de l'écran. Le projet est écrit en C # et convient parfaitement pour montrer la configuration du lancement de l'analyseur statique. Le code source du projet est
disponible sur GitHub .
La sortie de la commande cloc pour le projet ShareX:
En d'autres termes, le projet est petit, mais tout à fait suffisant pour démontrer le travail de PVS-Studio avec la plate-forme cloud.
Commençons la configuration
Pour commencer à travailler dans Azure DevOps, suivons le
lien et appuyez sur "Commencer gratuitement avec GitHub".
Accordez à l'application Microsoft l'accès aux données du compte GitHub.
Vous devrez créer un compte Microsoft pour terminer votre inscription.
Après l'enregistrement, créez un projet:
Ensuite, nous devons passer à «Pipelines» - «Builds» et créer un nouveau pipeline de build.
Lorsqu'on vous demandera où se trouve notre code, nous vous répondrons - GitHub.
Autorisez Azure Pipelines et choisissez le référentiel avec le projet, pour lequel nous allons configurer l'exécution de l'analyseur statique.
Dans la fenêtre de sélection du modèle, choisissez «Pipeline de démarrage».
Nous pouvons exécuter l'analyse de code statique du projet de deux manières: en utilisant des agents hébergés par Microsoft ou auto-hébergés.
Tout d'abord, nous utiliserons des agents hébergés par Microsoft. Ces agents sont des machines virtuelles ordinaires qui se lancent lorsque nous exécutons notre pipeline. Ils sont supprimés lorsque la tâche est terminée. L'utilisation de tels agents nous permet de ne pas perdre de temps pour leur support et leur mise à jour, mais impose certaines restrictions, par exemple - l'impossibilité d'installer des logiciels supplémentaires utilisés pour construire un projet.
Remplaçons la configuration par défaut suggérée pour la suivante pour l'utilisation d'agents hébergés par Microsoft:
# Setting up run triggers # Run only for changes in the master branch trigger: - master # Since the installation of random software in virtual machines # is prohibited, we'll use a Docker container, # launched on a virtual machine with Windows Server 1803 pool: vmImage: 'win1803' container: microsoft/dotnet-framework:4.7.2-sdk-windowsservercore-1803 steps: # Download the analyzer distribution - 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: | # Restore the project and download dependencies nuget restore .\ShareX.sln # Create the directory, where files with analyzer reports will be saved md .\PVSTestResults # Install the analyzer PVS-Studio_setup.exe /VERYSILENT /SUPPRESSMSGBOXES /NORESTART /COMPONENTS=Core # Create the file with configuration and license information "C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe" credentials -u $(PVS_USERNAME) -n $(PVS_KEY) # Run the static analyzer and convert the report in 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 # Save analyzer reports - task: PublishBuildArtifacts@1 inputs: pathToPublish: PVSTestResults artifactName: PVSTestResults
Remarque: selon la
documentation , le conteneur utilisé doit être mis en cache dans l'image de la machine virtuelle, mais au moment de la rédaction de l'article, il ne fonctionne pas et le conteneur est téléchargé à chaque démarrage de la tâche, ce qui a un impact négatif sur le calendrier d'exécution.
Sauvegardons le pipeline et créons des variables qui seront utilisées pour créer le fichier de licence. Pour ce faire, ouvrez la fenêtre d'édition du pipeline et cliquez sur "Variables" dans le coin supérieur droit.
Ensuite, ajoutez deux variables -
PVS_USERNAME et
PVS_KEY , contenant respectivement le nom d'utilisateur et la clé de licence. Lors de la création de la variable
PVS_KEY , n'oubliez pas de sélectionner "Garder cette valeur secrète" pour crypter les valeurs de la variable avec une clé RSA 2048 bits et pour supprimer la sortie de la valeur de variable dans le journal des performances de la tâche.
Enregistrez les variables et exécutez le pipeline en cliquant sur "Exécuter".
La deuxième option pour exécuter l'analyse - utilisez un agent auto-hébergé. Nous pouvons personnaliser et gérer nous-mêmes les agents auto-hébergés. Ces agents offrent plus de possibilités d'installer des logiciels, nécessaires pour créer et tester notre produit logiciel.
Avant d'utiliser de tels agents, vous devez les configurer conformément aux
instructions et installer et
configurer l'analyseur statique.
Pour exécuter la tâche sur un agent auto-hébergé, nous remplacerons la configuration suggérée par ce qui suit:
# Setting up triggers # Run the analysis for master-branch trigger: - master # The task is run on a self-hosted agent from the pool 'MyPool' pool: 'MyPool' steps: - task: CmdLine@2 inputs: workingDirectory: $(System.DefaultWorkingDirectory) script: | # Restore the project and download dependencies nuget restore .\ShareX.sln # Create the directory where files with analyzer reports will be saved md .\PVSTestResults # Run the static analyzer and convert the report in 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 # Save analyzer reports - task: PublishBuildArtifacts@1 inputs: pathToPublish: PVSTestResults artifactName: PVSTestResults
Une fois la tâche terminée, vous pouvez télécharger l'archive avec les rapports de l'analyseur sous l'onglet «Résumé» ou vous pouvez utiliser l'extension
Send Mail qui permet de configurer l'envoi d'e-
mails ou d'envisager un autre outil pratique sur
Marketplace .
Résultats d'analyse
Examinons maintenant quelques bogues trouvés dans le projet testé, ShareX.
Contrôles excessifsPour s'échauffer, commençons par de simples failles dans le code, à savoir, avec des contrôles redondants:
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; } .... }
Avertissement PVS-Studio: V3022 [CWE-571] L'expression 'dataObject! = Null' est toujours vraie. TaskThumbnailPanel.cs 415
Prenons attention à la vérification de la variable
dataObject pour
null . Pourquoi est-ce ici?
dataObject ne peut pas être
null dans ce cas, car il est initialisé par une référence sur un objet créé. En conséquence, nous avons un contrôle excessif. Critique? Non. Semble succinct? Non. Il est préférable de supprimer cette vérification pour ne pas encombrer le code.
Regardons un autre fragment de code que nous pouvons commenter de la même manière:
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); } } } .... }
Avertissement PVS-Studio: V3022 [CWE-571] L'expression 'img! = Null' est toujours vraie. ClipboardHelpers.cs 289
Dans la méthode
GetImageAlternative , la variable
img est vérifiée qu'elle n'est pas nulle juste après la création d'une nouvelle instance de la classe
Bitmap . La différence avec l'exemple précédent ici est que nous utilisons la méthode
GetDIBImage au lieu du constructeur pour initialiser la variable
img . L'auteur du code suggère qu'une exception pourrait se produire dans cette méthode, mais il déclare uniquement les blocs
try et
enfin , en omettant
catch . Par conséquent, si une exception se produit, la méthode d'appelant
GetImageAlternative n'obtiendra pas de référence à un objet du type
Bitmap , mais devra gérer l'exception dans son propre bloc
catch . Dans ce cas, la variable
img ne sera pas initialisée et le thread d'exécution n'atteindra même pas le contrôle
img! = Null mais entrera dans le bloc catch. Par conséquent, l'analyseur a indiqué un contrôle excessif.
Prenons l'exemple suivant d'un avertissement
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; } .... }
Avertissement PVS-Studio: V3022 [CWE-571] L'expression 'lvClipboardFormats.SelectedItems.Count> 0' est toujours vraie. AfterUploadForm.cs 155
Examinons de plus près la deuxième expression conditionnelle. Là, nous vérifions la valeur de la propriété
Count en lecture seule. Cette propriété affiche le nombre d'éléments dans l'instance de la collection
SelectedItems . La condition n'est exécutée que si la propriété
Count est supérieure à zéro. Tout irait bien, mais dans l'instruction
if externe,
Count est déjà vérifié pour 0. L'instance de la collection
SelectedItems ne peut pas avoir le nombre d'éléments inférieur à zéro, par conséquent,
Count est égal ou supérieur à 0. Puisque nous avons déjà effectué la vérification
Count pour 0 dans la première instruction
if et c'était faux, il est inutile d'écrire une autre vérification
Count pour être supérieur à zéro dans la branche else.
Le dernier exemple d'un avertissement
V3022 sera le fragment de code suivant:
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; .... } .... }
Avertissement PVS-Studio: l' expression
V3022 'itemCount> 0' est toujours fausse. RegionCaptureForm.cs 1100
L'analyseur a remarqué que la condition
itemCount> 0 sera toujours fausse, car la variable
itemCount est déclarée et en même temps assignée zéro ci-dessus. Cette variable n'est utilisée nulle part jusqu'à la condition même, donc l'analyseur avait raison sur l'expression conditionnelle, dont la valeur est toujours fausse.
Eh bien, regardons maintenant quelque chose de vraiment sapide.
La meilleure façon de comprendre un bug est de visualiser un bugIl nous semble qu'une erreur assez intéressante a été trouvée à cet endroit:
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)); .... }
Je ne voudrais pas montrer toutes les cartes et révéler ce que notre analyseur a trouvé, alors mettons-le de côté pendant un moment.
Par le nom de la méthode, il est facile de deviner ce qu'elle fait - vous lui donnez une image ou un fragment d'une image, et elle la pixellise. Le code de la méthode est assez long, donc nous ne le citerons pas entièrement, mais essayons simplement d'expliquer son algorithme et d'expliquer quel genre de bogue PVS-Studio a réussi à trouver.
Cette méthode reçoit deux paramètres: un objet de type
Bitmap et la valeur de type
int qui indique la taille de la pixellisation. L'algorithme de fonctionnement est assez simple:
1) Divisez le fragment d'image reçu en carrés avec le côté égal à la taille de la pixellisation. Par exemple, si nous avons une taille de pixelation égale à 15, nous aurons un carré contenant 15x15 = 225 pixels.
2) De plus, nous parcourons chaque pixel de ce carré et accumulons les valeurs des champs
Rouge ,
Vert ,
Bleu et
Alpha dans des variables intermédiaires, et avant cela multiplions la valeur de la couleur correspondante et du canal alpha par la variable
pixelWeight , obtenue par divisant la valeur
Alpha par 255 (la variable
Alpha est du type
octet ). De plus, lors de la traversée de pixels, nous
résumons les valeurs écrites en
pixelWeight dans la variable
weightedCount . Le fragment de code qui exécute les actions ci-dessus est le suivant:
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;
Soit dit en passant, si la valeur de la variable
Alpha est nulle,
pixelWeight n'ajoutera à la variable
weightedCount aucune valeur pour ce pixel. Nous en aurons besoin à l'avenir.
3) Après avoir traversé tous les pixels du carré actuel, nous pouvons faire une couleur "moyenne" commune pour ce carré. Le code faisant cela ressemble à ceci:
ColorBgra averageColor = new ColorBgra((byte)(b / weightedCount), (byte)(g / weightedCount), (byte)(r / weightedCount), (byte)(a / pixelCount));
4) Maintenant, lorsque nous avons obtenu la couleur finale et l'
avons écrite dans la variable
averageColor , nous pouvons à nouveau parcourir chaque pixel du carré et lui affecter une valeur de
averageColor .
5) Revenons au point 2 pendant que nous avons des carrés non gérés.
Encore une fois, la variable
weightedCount n'est pas égale au nombre de tous les pixels d'un carré. Par exemple, si une image contient un pixel complètement transparent (valeur nulle dans le canal alpha), la variable
pixelWeight sera nulle pour ce pixel (
0/255 = 0). Par conséquent, ce pixel n'affectera pas la formation de la variable
weightedCount . C'est tout à fait logique - il est inutile de prendre en compte les couleurs d'un pixel complètement transparent.
Tout semble donc raisonnable - la pixellisation doit fonctionner correctement. Et c'est effectivement le cas. Ce n'est tout simplement pas pour les images png qui incluent des pixels avec des valeurs dans le canal alpha inférieures à 255 et différentes de zéro. Remarquez l'image pixélisée ci-dessous:
Avez-vous vu la pixellisation? Nous non plus. D'accord, révélons maintenant cette petite intrigue et expliquons où se cache exactement le bogue dans cette méthode. L'erreur s'est glissée dans la ligne de calcul de la variable
pixelWeight :
float pixelWeight = color.Alpha / 255;
Le fait est qu'en déclarant la variable
pixelWeight comme
float , l'auteur du code a laissé entendre qu'en divisant le champ
Alpha par 255, il obtiendrait des nombres fractionnaires en plus de zéro et un. C'est là que se cache le problème, car la variable
Alpha est du type
octet . Lorsque vous le plongez de 255, nous obtenons une valeur entière. Ce n'est qu'après cela qu'il sera implicitement
converti en type
flottant , ce qui signifie que la partie fractionnaire sera perdue.
Il est facile d'expliquer pourquoi il est impossible de pixelliser des images png avec une certaine transparence. Étant donné que pour ces pixels, les valeurs du canal alpha sont dans la plage 0 <Alpha <255, la variable
Alpha divisée par 255 donnera toujours 0. Par conséquent, les valeurs des variables
pixelWeight ,
r ,
g ,
b ,
a ,
weightedCount seront également toujours être 0. Par conséquent, notre
couleur moyenne aura des valeurs nulles dans tous les canaux: rouge - 0, bleu - 0, vert - 0, alpha - 0. En peignant un carré de cette couleur, nous ne changeons pas la couleur d'origine des pixels, car la
couleur moyenne est absolument transparente. Pour corriger cette erreur, nous avons juste besoin de convertir explicitement le champ
Alpha en type
flottant . La version fixe de la ligne de code pourrait ressembler à ceci:
float pixelWeight = (float)color.Alpha / 255;
Eh bien, il est grand temps de citer le message de PVS-Studio pour le code incorrect:
Avertissement PVS-Studio: V3041 [CWE-682] L'expression a été implicitement
convertie du type 'int' en type 'float'. Pensez à utiliser un transtypage de type explicite pour éviter la perte d'une partie fractionnaire. Un exemple: double A = (double) (X) / Y;. ImageHelpers.cs 1119
A titre de comparaison, citons la capture d'écran d'une image vraiment pixélisée, obtenue sur la version corrigée de l'application:
Potentiel NullReferenceException public static bool AddMetadata(Image img, int id, string text) { .... pi.Value = bytesText; if (pi != null) { img.SetPropertyItem(pi); return true; } .... }
Avertissement PVS-Studio: V3095 [CWE-476] L'objet 'pi' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 801, 803. ImageHelpers.cs 801
Ce fragment de code montre que l'auteur s'attendait à ce que la variable
pi puisse être
nulle , c'est pourquoi avant d'appeler la méthode
SetPropertyItem , la vérification
pi! = Null a lieu. Il est étrange qu'avant cette vérification, la propriété se voit attribuer un tableau d'octets, car si
pi est
nul , une exception du type
NullReferenceException sera levée.
Une situation similaire a été constatée dans un autre endroit:
private static void Task_TaskCompleted(WorkerTask task) { .... task.KeepImage = false; if (task != null) { if (task.RequestSettingUpdate) { Program.MainForm.UpdateCheckStates(); } .... } .... }
Avertissement PVS-Studio: V3095 [CWE-476] L'objet 'tâche' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 268, 270. TaskManager.cs 268
PVS-Studio a trouvé une autre erreur similaire. Le point est le même, il n'y a donc pas grand besoin de citer le fragment de code, le message de l'analyseur sera suffisant.
Avertissement PVS-Studio: V3095 [CWE-476] L'objet 'Config.PhotobucketAccountInfo' a été utilisé avant d'être vérifié par rapport à null. Vérifier les lignes: 216, 219. UploadersConfigForm.cs 216
La même valeur de retourUn fragment de code suspect a été trouvé dans la méthode
EvalWindows de la classe
WindowsList , qui retourne
vrai dans tous les cas:
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;
Avertissement PVS-Studio: V3009 Il est étrange que cette méthode renvoie toujours une seule et même valeur de «vrai». WindowsList.cs 82
Il semble logique que si dans la liste nommée
IgnoreWindows il y a un pointeur avec le même nom que
hWnd , la méthode doit retourner
false .
La liste
IgnoreWindows peut être remplie lors de l'appel du constructeur
WindowsList (IntPtr ignoreWindow) ou directement en accédant à la propriété car elle est publique. Quoi qu'il en soit, selon Visual Studio, pour le moment dans le code, cette liste n'est pas remplie. C'est un autre endroit étrange de cette méthode.
Appel dangereux des gestionnaires d'événements protected void OnNewsLoaded() { if (NewsLoaded != null) { NewsLoaded(this, EventArgs.Empty); } }
Avertissement PVS-Studio: V3083 [CWE-367] L'appel non sécurisé de l'événement 'NewsLoaded', NullReferenceException est possible. Pensez à affecter un événement à une variable locale avant de l'invoquer. NewsListControl.cs 111
Ici, un cas très méchant pourrait se produire. Après avoir vérifié la
null de la variable
NewsLoaded , la méthode, qui gère un événement, peut être désinscrite, par exemple, dans un autre thread. Dans ce cas, au moment où nous entrons dans le corps de l'instruction if, la variable
NewsLoaded sera déjà nulle. Une
exception NullReferenceException peut se produire lorsque vous essayez d'appeler des abonnés à partir de l'événement
NewsLoaded , qui est null. Il est beaucoup plus sûr d'utiliser un opérateur à condition nulle et de réécrire le code ci-dessus comme suit:
protected void OnNewsLoaded() { NewsLoaded?.Invoke(this, EventArgs.Empty); }
L'analyseur a identifié
68 fragments similaires. Nous ne les décrirons pas tous - ils ont tous un modèle d'appel similaire.
Retourne null de ToStringRécemment, j'ai découvert dans un
article intéressant de mon collègue que Microsoft ne recommande pas de retourner null à partir de la méthode remplacée
ToString . PVS-Studio en est bien conscient:
public override string ToString() { lock (loggerLock) { if (sbMessages != null && sbMessages.Length > 0) { return sbMessages.ToString(); } return null; } }
Avertissement PVS-Studio: V3108 Il n'est pas recommandé de renvoyer 'null' à partir de la méthode 'ToSting ()'. Logger.cs 167
Pourquoi attribué si non utilisé? public SeafileCheckAccInfoResponse GetAccountInfo() { string url = URLHelpers.FixPrefix(APIURL); url = URLHelpers.CombineURL(APIURL, "account/info/?format=json"); .... }
Avertissement PVS-Studio: V3008 La variable 'url' se voit attribuer des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 197, 196. Seafile.cs 197
Comme nous pouvons le voir dans l'exemple, lors de la déclaration de la variable
url , une valeur lui est affectée, renvoyée par la méthode
FixPrefix . Dans la ligne suivante, nous effaçons la valeur obtenue même sans l'utiliser nulle part. Nous obtenons quelque chose de similaire au code mort: cela fonctionne, mais n'affecte pas le résultat. Très probablement, cette erreur est le résultat d'un copier-coller, car de tels fragments de code ont lieu dans 9 autres méthodes. À titre d'exemple, nous citerons deux méthodes avec une première ligne similaire:
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"); .... }
Conclusions
Comme nous pouvons le voir, la complexité de la configuration des vérifications automatiques de l'analyseur ne dépend pas du système CI choisi. Cela nous a pris littéralement 15 minutes et plusieurs clics de souris pour configurer la vérification de notre code de projet avec un analyseur statique.
En conclusion, nous vous invitons à
télécharger et essayer l'analyseur sur vos projets.