Aujourd'hui, nous avons un autre projet Microsoft de haute qualité à vérifier, que nous allons approfondir en essayant de trouver des erreurs avec PVS-Studio. SARIF, acronyme de Static Analysis Interchange Format, qui est un standard (format de fichier), conçu pour interagir et partager les résultats des analyseurs statiques avec d'autres outils: IDE, outils complexes de vérification et d'analyse de code (par exemple SonarQube), systèmes d'intégration continue, etc. Le SDK SARIF, respectivement, contient des outils de développement .NET pour prendre en charge SARIF ainsi que des fichiers supplémentaires.
SARIF est originaire de Microsoft et est maintenant une norme développée par OASIS (un consortium à but non lucratif qui traite des normes ouvertes). SARIF est censé transmettre non seulement les résultats de l'analyseur, mais également les métadonnées sur l'outil, ainsi que les données sur la façon dont il a été lancé, les balises temporelles, etc. Pour plus d'informations, visitez le site Web d'
OASIS . Le code source du SDK SARIF peut être téléchargé à partir du référentiel sur
GiHub . La page d'accueil du projet est disponible par
lien .
À propos du projet
Le projet SARIF SDK s'est avéré être petit: 799 fichiers .cs (environ 98 000 lignes de code non vides). Le projet contient des tests que j'exclus toujours de la vérification. Ainsi, la partie du code qui nous intéressait était 642 fichiers .cs (environ 79 000 lignes de code non vides). Ce n'est certainement pas suffisant. Sur le plan positif, la vérification et l'analyse ont été faciles et rapides, entre ce moment et ensuite, que j'ai essayé de refléter sur l'image au début. Néanmoins, j'ai réussi à retrouver certains cas étranges. Jetons-y un œil.
Des erreurs
V3070 [CWE-457] La variable 'Binary' non initialisée est utilisée lors de l'initialisation de la variable 'Default'. MimeType.cs 90
public static class MimeType { ....
Le champ est initialisé par la valeur d'un autre champ, qui n'a pas encore reçu de valeur. Par conséquent,
Default recevra la valeur nulle par défaut pour le type de
chaîne . Très probablement, l'erreur est restée inaperçue, car le champ
Par défaut n'est utilisé nulle part. Mais les choses peuvent changer, puis le développeur sera confronté à un résultat indu ou au plantage du programme.
V3061 Le paramètre 'logicLocationToIndexMap' est toujours réécrit dans le corps de la méthode avant d'être utilisé. PrereleaseCompatibilityTransformer.cs 1963
private static JArray ConvertLogicalLocationsDictionaryToArray( .... Dictionary<LogicalLocation, int> logicalLocationToIndexMap, ....) { .... logicalLocationToIndexMap = new Dictionary<LogicalLocation, int>(LogicalLocation.ValueComparer); .... }
L'auteur du code n'utilise en aucune façon le paramètre
logicLocationToIndexMap , mais y écrit une valeur différente. Curieusement, la valeur précédente est exactement le même dictionnaire vide, créé dans le code de l'appelant:
private static bool ApplyChangesFromTC25ThroughTC30(....) { .... Dictionary<LogicalLocation, int> logicalLocationToIndexMap = null; .... logicalLocationToIndexMap = new Dictionary<LogicalLocation, int>(LogicalLocation.ValueComparer); run["logicalLocations"] = ConvertLogicalLocationsDictionaryToArray( ...., logicalLocationToIndexMap, ....); }
Code étrange et suspect.
V3008 [CWE-563] La variable 'run.Tool' reçoit des valeurs successives deux fois. C'est peut-être une erreur. Vérifiez les lignes: 116, 114. ExportRulesMetadataCommandBase.cs 116
public partial class Run { .... public Tool Tool { get; set; } .... } public partial class Tool : .... { .... public Tool() { } .... } private void OutputSarifRulesMetada(....) { .... var run = new Run(); run.Tool = new Tool(); run.Tool = Tool.CreateFromAssemblyData(....);
La propriété
run.Tool se voit attribuer une valeur deux fois. Tant lors de la création de l'objet
Tool que lors de l'écriture d'une valeur dans la propriété
Tool , aucun travail supplémentaire n'est requis. Par conséquent, la réaffectation sent le poisson.
V3042 [CWE-476] Possible NullReferenceException. Le '?.' et '.' les opérateurs sont utilisés pour accéder aux membres de l'objet «loc» WhereComparer.cs 152
private static Uri ArtifactUri(ArtifactLocation loc, Run run) { return loc?.Uri ?? loc.Resolve(run)?.Uri; }
Si la valeur de la variable
loc est
nulle , une tentative sera faite pour renvoyer la valeur de la partie droite du ?? , entraînant l'accès par référence nulle.
V3042 [CWE-476] Possible NullReferenceException. Le '?.' et '.' les opérateurs sont utilisés pour accéder aux membres de l'objet 'formatString' InsertOptionalDataVisitor.cs 194
public override Message VisitMessage(Message node) { .... node.Text = node.Arguments?.Count > 0 ? string.Format(...., formatString.Text, ....) : formatString?.Text; .... }
Les développeurs utilisent des options d'accès non sécurisées et sécurisées par une référence
formatString potentiellement nulle dans deux branches parallèles de l'opérateur conditionnel ?:.
V3042 [CWE-476] Possible NullReferenceException. Le '?.' et '.' les opérateurs sont utilisés pour accéder aux membres de l'objet 'messageText' FortifyFprConverter.cs 1210
V3042 [CWE-476] Possible NullReferenceException. Le '?.' et '.' les opérateurs sont utilisés pour accéder aux membres de l'objet 'messageText' FortifyFprConverter.cs 1216
private void AddMessagesToResult(Result result) { .... string messageText = (rule.ShortDescription ?? rule.FullDescription)?.Text; .... if (....) {
Ici, l'analyseur a déjà émis deux avertissements concernant un accès possible par la référence null
messageText . Cela semble plutôt non menaçant, mais c'est toujours une erreur.
V3080 [CWE-476]
Déréférence nulle possible. Pensez à inspecter «fileDataVersionOne.Uri». SarifCurrentToVersionOneVisitor.cs 1030
private IDictionary<string, FileDataVersionOne> CreateFileDataVersionOneDictionary() { .... FileDataVersionOne fileDataVersionOne = CreateFileDataVersionOne(v2File); if (fileDataVersionOne.Uri.OriginalString.Equals(key)) { .... } .... }
L'analyseur soupçonne que
NullReferenceException est possible lors de l'
utilisation de la référence
fileDataVersionOne.Uri . Voyons d'où vient cette variable et découvrons si l'analyseur a raison. Pour ce faire, examinons de près le corps de la méthode
CreateFileDataVersionOne :
private FileDataVersionOne CreateFileDataVersionOne(Artifact v2FileData) { FileDataVersionOne fileData = null; if (v2FileData != null) { .... fileData = new FileDataVersionOne { .... Uri = v2FileData.Location?.Uri, .... }; .... } return fileData; } public partial class FileDataVersionOne { .... public Uri Uri { get; set; } .... }
En effet, lors de la création de l'objet de la classe
FileDataVersionOne , la propriété
Uri peut recevoir la valeur
nulle . Il s'agit d'un excellent exemple d'analyse de flux de données et de mécanismes d'analyse interprocédurale fonctionnant ensemble.
V3080 [CWE-476]
Déréférence nulle possible. Pensez à inspecter '_jsonTextWriter'. SarifLogger.cs 242
public virtual void Dispose() { .... if (_closeWriterOnDispose) { if (_textWriter != null) { _textWriter.Dispose(); } if (_jsonTextWriter == null) { _jsonTextWriter.Close(); }
Il y a une faute de frappe dans ce fragment. Il est clair que
_jsonTextWriter! = Null doit être dans l'état du deuxième bloc. Ce morceau de code est en danger car, très probablement, il ne
plante pas, car
_jsonTextWriter n'est pas
nul . En outre, le flux reste ouvert.
V3083 [CWE-367] L'appel non sécurisé de l'événement 'RuleRead', NullReferenceException est possible. Pensez à affecter un événement à une variable locale avant de l'invoquer. FxCopConverter.cs 897
private void ReadRule(....) { .... if (RuleRead != null) { RuleRead(....); } .... }
Les événements sont traités de manière non sécurisée. Il s'agit d'un bogue non critique qui peut être facilement corrigé, par exemple, en suivant l'astuce de Visual Studio. Voici le remplacement suggéré par l'IDE:
private void ReadRule(....) { .... RuleRead?.Invoke(....); .... }
Cela ne prend que quelques secondes pour le réparer, mais l'analyseur ne s'en plaindra plus et l'IDE ne mettra pas en évidence le code. Une autre erreur similaire.
- V3083 [CWE-367] L'appel non sécurisé de l'événement 'ResultRead', NullReferenceException est possible. Pensez à affecter un événement à une variable locale avant de l'invoquer. FxCopConverter.cs 813
V3095 [CWE-476] L'objet 'v1Location' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 333, 335. SarifVersionOneToCurrentVisitor.cs 333
internal Location CreateLocation(LocationVersionOne v1Location) { .... string key = v1Location.LogicalLocationKey ?? v1Location.FullyQualifiedLogicalName; if (v1Location != null) { .... } .... }
L'auteur pensait que la référence
v1Location pouvait être nulle et a ajouté une vérification appropriée. Alors que ci-dessus, nous pouvons voir que cette référence est gérée sans aucun contrôle. Refactorisation inattentive? Eh bien, on ne sait jamais.
V3125 [CWE-476] L'objet 'v1StackFrame' a été utilisé après avoir été vérifié par rapport à null. Vérifiez les lignes: 1182, 1171. SarifVersionOneToCurrentVisitor.cs 1182
internal StackFrame CreateStackFrame(StackFrameVersionOne v1StackFrame) { StackFrame stackFrame = null; if (v1StackFrame != null) { stackFrame = new StackFrame { .... }; } stackFrame.Location = CreateLocation(v1StackFrame.FullyQualifiedLogicalName, v1StackFrame.LogicalLocationKey, ....); return stackFrame; }
Comme toujours, voici un cas inverse. D'abord, la référence
v1StackFrame est vérifiée pour
null , puis la vérification est égarée. Mais ce cas a une mise en garde importante: les variables
v1StackFrame et
stackFrame sont logiquement liées. Voir, si
v1StackFrame est
null , l'objet
StackFrame ne sera pas créé, tandis que
stackFrame restera
nul. Suivi du plantage du programme en raison d'un appel de
stackFrame.Location , car il n'y a aucune vérification ici. Il ne sera donc même pas question de l'utilisation dangereuse de
v1StackFrame , indiquée par l'analyseur. Ce code ne fonctionne que si vous transmettez des valeurs
v1StackFrame non
nulles à la méthode
CreateStackFrame . Je soupçonnais que le code de l'appelant le contrôlait d'une manière ou d'une autre.
Les appels CreateStackFrame ressemblent à ceci:
Frames = v1Stack.Frames?.Select(CreateStackFrame).ToList()
CreateStackFrame est utilisé comme sélecteur. Les références passées ne sont pas vérifiées pour
null ici. Peut-être que lors du remplissage de la collection
Frames , elle (l'écriture de références nulles) est contrôlée, mais je ne suis pas allé trop loin. La conclusion est déjà évidente - le code requiert l'attention des auteurs.
Conclusion
Comme vous le voyez, l'article n'est pas long mais j'espère que vous avez apprécié cette lecture légère :) Au cas où, vous pouvez toujours
télécharger notre analyseur pour rechercher vous-même les erreurs dans vos projets ou ceux de quelqu'un.
Et enfin, une petite annonce: mon prochain article portera sur les erreurs les plus intéressantes que mes collègues et moi avons trouvées dans les projets en 2019. Suivez notre
blog . A bientôt!
Pour en savoir plus sur les nouveaux articles de blog, vous pouvez vous abonner aux chaînes suivantes: