About the project

Errors

public static class MimeType { .... /// <summary>The MIME type to use when no better MIME type is known.</summary> public static readonly string Default = Binary; .... /// <summary>The MIME type for binaries.</summary> public static readonly string Binary = "application/octet-stream"; .... }

private static JArray ConvertLogicalLocationsDictionaryToArray( .... Dictionary<LogicalLocation, int> logicalLocationToIndexMap, ....) { .... logicalLocationToIndexMap = new Dictionary<LogicalLocation, int>(LogicalLocation.ValueComparer); .... }

private static bool ApplyChangesFromTC25ThroughTC30(....) { .... Dictionary<LogicalLocation, int> logicalLocationToIndexMap = null; .... logicalLocationToIndexMap = new Dictionary<LogicalLocation, int>(LogicalLocation.ValueComparer); run["logicalLocations"] = ConvertLogicalLocationsDictionaryToArray( ...., logicalLocationToIndexMap, ....); }

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(....); // <= .... }

private static Uri ArtifactUri(ArtifactLocation loc, Run run) { return loc?.Uri ?? loc.Resolve(run)?.Uri; }

public override Message VisitMessage(Message node) { .... node.Text = node.Arguments?.Count > 0 ? string.Format(...., formatString.Text, ....) : formatString?.Text; .... }

private void AddMessagesToResult(Result result) { .... string messageText = (rule.ShortDescription ?? rule.FullDescription)?.Text; .... if (....) { // Replace the token with an embedded hyperlink. messageText = messageText.Replace(....); } else { // Replace the token with plain text. messageText = messageText.Replace(....); } .... }

private IDictionary<string, FileDataVersionOne> CreateFileDataVersionOneDictionary() { .... FileDataVersionOne fileDataVersionOne = CreateFileDataVersionOne(v2File); if (fileDataVersionOne.Uri.OriginalString.Equals(key)) { .... } .... }

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; } .... }

public virtual void Dispose() { .... if (_closeWriterOnDispose) { if (_textWriter != null) { _textWriter.Dispose(); } if (_jsonTextWriter == null) { _jsonTextWriter.Close(); } // <= } .... }

private void ReadRule(....) { .... if (RuleRead != null) { RuleRead(....); } .... }

private void ReadRule(....) { .... RuleRead?.Invoke(....); .... }

V3083 [CWE-367] Unsafe invocation of event 'ResultRead', NullReferenceException is possible. Consider assigning event to a local variable before invoking it. FxCopConverter.cs 813

internal Location CreateLocation(LocationVersionOne v1Location) { .... string key = v1Location.LogicalLocationKey ?? v1Location.FullyQualifiedLogicalName; if (v1Location != null) { .... } .... }

internal StackFrame CreateStackFrame(StackFrameVersionOne v1StackFrame) { StackFrame stackFrame = null; if (v1StackFrame != null) { stackFrame = new StackFrame { .... }; } stackFrame.Location = CreateLocation(v1StackFrame.FullyQualifiedLogicalName, v1StackFrame.LogicalLocationKey, ....); return stackFrame; }

Frames = v1Stack.Frames?.Select(CreateStackFrame).ToList()

Conclusion

Today we have another high-quality Microsoft project to be checked, which we'll heroically delve into trying to find errors with PVS-Studio. SARIF, an acronym for Static Analysis Interchange Format, which is a standard (file format), designed to interact and share the results of static analyzers with other tools: IDEs, complex code verification and analysis tools (e.g. SonarQube), continuous integration systems, etc. SARIF SDK, respectively, contains .NET developer tools to support SARIF as well as additional files.SARIF originated at Microsoft and is now a standard developed by OASIS (a non-profit consortium that deals with open standards). SARIF is meant to pass not only the results of the analyzer, but also metadata about the tool, as well as data on how it was launched, time tags, and so on. For more information, visit the OASIS website. The source code of SARIF SDK can be downloaded from the repository on GiHub . The project's homepage is available by link The SARIF SDK project turned out to be small: 799 .cs files (approximately 98,000 non-empty lines of code). The project contains tests that I always exclude from the check. Thus, the part of the code we were interested in was 642 .cs files (approximately 79,000 non-empty lines of code). It's certainly not enough. On the plus side, the check and analysis were easy and fast, between this and then, which I tried to reflect on the picture at the beginning. Nonetheless, I managed to track down some uncanny cases. Let's have a look at them. V3070 [CWE-457] Uninitialized variable 'Binary' is used when initializing the 'Default' variable. MimeType.cs 90The field is initialized by the value of another field, which hasn't received a value yet. As a result,will receive the null value by default for thetype. Most likely, the error remained unnoticed, as thefield isn't used anywhere. But things can change, and then the developer will face an undue result or the program crash. V3061 Parameter 'logicalLocationToIndexMap' is always rewritten in method body before being used. PrereleaseCompatibilityTransformer.cs 1963The code author doesn't use theparameter in any way, but writes a different value in it. Curiously enough, the previous value is exactly the same empty dictionary, created in the caller code:Weird and suspicious code. V3008 [CWE-563] The 'run.Tool' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 116, 114. ExportRulesMetadataCommandBase.cs 116Theproperty is assigned a value twice. Both when creating theobject and when writing a value in theproperty, no additional work is required. Therefore, reassigning smells fishy. V3042 [CWE-476] Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'loc' object WhereComparer.cs 152If the value of thevariable is, an attempt will be made to return the value from the right part of the ?? operator, resulting in the access by null reference. V3042 [CWE-476] Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'formatString' object InsertOptionalDataVisitor.cs 194Developers use unsecure and secure access variants by a potentially nullreference in two parallel branches of the conditional ?: operator. V3042 [CWE-476] Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'messageText' object FortifyFprConverter.cs 1210 V3042 [CWE-476] Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'messageText' object FortifyFprConverter.cs 1216Here the analyzer issued already two warnings about possible access by the nullreference. It looks rather nonthreatening, but it's still an error. V3080 [CWE-476] Possible null dereference. Consider inspecting 'fileDataVersionOne.Uri'. SarifCurrentToVersionOneVisitor.cs 1030The analyzer suspected thatis possible when working with thereference. Let's see where this variable comes from and find out if the analyzer is right. To do this, let's take a close look at the body of themethod:Indeed, when creating the object of theclass, theproperty might receive thevalue. This is a great example data flow analysis and interprocedural analysis mechanisms working together. V3080 [CWE-476] Possible null dereference. Consider inspecting '_jsonTextWriter'. SarifLogger.cs 242There's a typo in this fragment. It's clear thathas to be in the condition of the second block. This piece of code is jeopardizing as, most likely, it doesn't crash, due tobeing. Besides, the stream remains open. V3083 [CWE-367] Unsafe invocation of event 'RuleRead', NullReferenceException is possible. Consider assigning event to a local variable before invoking it. FxCopConverter.cs 897Events are handled unsafely. It's an uncritical bug that can be easily fixed, for example, by following the Visual Studio tip. Here's the replacement suggested by the IDE:It takes only a few seconds to fix it, but the analyzer will no longer complain about it and the IDE won't highlight the code. Another similar error. V3095 [CWE-476] The 'v1Location' object was used before it was verified against null. Check lines: 333, 335. SarifVersionOneToCurrentVisitor.cs 333The author thought that thereference may be null and added an appropriate check. Whereas above we can see that this reference is handled without any checks. Inattentive refactoring? Well, you never know. V3125 [CWE-476] The 'v1StackFrame' object was used after it was verified against null. Check lines: 1182, 1171. SarifVersionOneToCurrentVisitor.cs 1182As always, here comes a reverse case. First thereference is checked for, and then the check is gone astray. But this case has an important caveat:andvariables are logically related. See, ifis, theobject won't be created, whereaswill remainFollowed by the program crash due to a call of, as there are no checks here. So it won't even come to the dangeroususage, indicated by the analyzer. This code only works if you pass nonnullvalues to themethod. I suspected that the caller code somehow controls it.callslook like this:is used as a selector. Passed references aren't checked forhere. Perhaps, when filling thecollection it (writing of null references) is controlled, but I didn't go for digging too deep. The conclusion is already obvious — the code requires authors' attention.As you see, the article isn't long but I hope you enjoyed this light reading :) Just in case, that you can always download our analyzer to search for errors in your or someone's projects yourself.And finally, a small announcement: my next article will be about the most interesting errors that my colleagues and I found in projects in 2019. Follow our blog . See you!To learn more about new blog posts, you are welcome to subscribe to the following channels: