Weaknesses detected by PVS-Studio this week: episode N1

Andrey Karpov
Articles: 326



We decided to search and fix potential vulnerabilities in various projects. You can call this as you wish - some kind of help to open source projects; a method of promotion or testing of the analyzer. Another way to see it as a way to attract attention to the reliability and quality of the code. In fact, the way to name these posts does not really matter - we just like doing it. This is our little hobby. So, let us have a look at our findings in the code of various projects this week - we had some time to make fixes and suggest looking at them.

Picture 1

For those who are not familiar with PVS-Studio tool

PVS-Studio is a tool that detects a large number of types of vulnerabilities and errors in the code. It performs static analysis and points to code fragments that are likely to contain errors. The best effect is achieved when the static analysis is performed regularly. Ideologically, the analyzer warnings are similar to the compiler warnings. However, unlike compilers, PVS-Studio can perform deeper and more versatile code analysis. This enables it to detect errors, even in compilers: GCC; LLVM 1, 2, 3; Roslyn.

The tool supports the analysis of C, C++ and C#; works under Windows and Linux. The analyzer can be integrated as a Visual Studio plug-in.

We suggest the following materials for further investigation of the tool:

Weaknesses

In this section we show those defects that fall under the CWE classification and are potential vulnerabilities in their core. Of course, not all weaknesses are really threatening for a project, but we wanted to show that our tool is able to detect them.

1. CoreFX. CWE-476 (NULL Pointer Dereference)

V3080 Possible null dereference. Consider inspecting '_swtFirst'. MemberLookup.cs 109

if (_swtFirst == null)
{
  _swtFirst.Set(sym, type); // <= 
  ....
}

Pull Request: https://github.com/dotnet/corefx/pull/16807

2. CoreFX. CWE-476 (NULL Pointer Dereference)

V3080 Possible null dereference. Consider inspecting 'tabClasses'. PropertyTabAttribute.cs 225

if (tabClasses != null)                        // <=
{
  if (tabScopes != null && tabClasses.Length != tabScopes.Length)
  {
    ....
  }
  _tabClasses = (Type[])tabClasses.Clone();
}
else if (tabClassNames != null)
{
  if (tabScopes != null &&
      tabClasses.Length != tabScopes.Length)    // <=
  {
    ....
  }
  _tabClassNames = (string[])tabClassNames.Clone();
  _tabClasses = null;
}

Pull Request: https://github.com/dotnet/corefx/pull/16807

3. CoreFX. CWE-476 (NULL Pointer Dereference)

V3080 Possible null dereference. Consider inspecting 'BaseSimpleType'. SimpleType.cs 368

if ((BaseSimpleType == null && otherSimpleType.BaseSimpleType != null)
    &&
    (BaseSimpleType.HasConflictingDefinition(...)).Length != 0) // <=
    return ("BaseSimpleType");

Pull Request: https://github.com/dotnet/corefx/pull/16807

4. CoreFX. CWE-476 (NULL Pointer Dereference)

V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'o', 'other'. CompilerInfo.cs 106

CompilerInfo other = o as CompilerInfo;
if (o == null)
{
    return false;
}
return CodeDomProviderType == other.CodeDomProviderType && ... // <=

Pull Request: https://github.com/dotnet/corefx/pull/16807

5. CoreFX. CWE-476 (NULL Pointer Dereference)

V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'myObject', 'myString'. CaseInsensitiveAscii.cs 46

string myString = myObject as string;
if (myObject == null)
{
    return 0;
}
int myHashCode = myString.Length;      // <=

PVS-Studio: fixed vulnerability CWE-476 (NULL Pointer Dereference)

Pull Request: https://github.com/dotnet/corefx/pull/16807

6. CoreFX. CWE-476 (NULL Pointer Dereference)

V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'a', 'nodeA'. AttributeSortOrder.cs 22

V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'b', 'nodeB'. AttributeSortOrder.cs 22

XmlNode nodeA = a as XmlNode;
XmlNode nodeB = b as XmlNode;
if ((a == null) || (b == null))
    throw new ArgumentException();
int namespaceCompare =
  string.CompareOrdinal(nodeA.NamespaceURI, nodeB.NamespaceURI); // <=

Pull Request: https://github.com/dotnet/corefx/pull/16807

7. CoreFX. CWE-476 (NULL Pointer Dereference)

V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'a', 'nodeA'. NamespaceSortOrder.cs 21

V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'b', 'nodeB'. NamespaceSortOrder.cs 21

XmlNode nodeA = a as XmlNode;
XmlNode nodeB = b as XmlNode;
if ((a == null) || (b == null))
    throw new ArgumentException();
bool nodeAdefault = Utils.IsDefaultNamespaceNode(nodeA); 
bool nodeBdefault = Utils.IsDefaultNamespaceNode(nodeB); 

Pull Request: https://github.com/dotnet/corefx/pull/16807

8. MSBuild. CWE-476 (NULL Pointer Dereference)

V3095 The 'name' object was used before it was verified against null. Check lines: 229, 235. Microsoft.Build.Tasks GenerateBindingRedirects.cs 229

V3095 The 'publicKeyToken' object was used before it was verified against null. Check lines: 231, 235. Microsoft.Build.Tasks GenerateBindingRedirects.cs 231

private void UpdateExistingBindingRedirects(....)
{
  ....
  var name = assemblyIdentity.Attribute("name");
  var nameValue = name.Value;  // <=
  var publicKeyToken = assemblyIdentity.
                       Attribute("publicKeyToken");
  var publicKeyTokenValue = publicKeyToken.Value;  // <=
  var culture = assemblyIdentity.Attribute("culture");
  var cultureValue = culture == null ? 
                     String.Empty : culture.Value;
  
  if (name == null || publicKeyToken == null)
  {
      continue;
  }  
  ....
}

Pull Request: https://github.com/Microsoft/msbuild/pull/1829

Miscellaneous errors

1. MSBuild

V3041 The expression was implicitly cast from 'long' type to 'float' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. Microsoft.Build CommunicationsUtilities.cs 615

private static long s_lastLoggedTicks = DateTime.UtcNow.Ticks;
internal static void Trace(....)
{
  ....
  long now = DateTime.UtcNow.Ticks;
  float millisecondsSinceLastLog = 
    (float)((now - s_lastLoggedTicks) / 10000L);
  ....
}

Pull Request: https://github.com/Microsoft/msbuild/pull/1829

2. MSBuild

V3118 Milliseconds component of TimeSpan is used, which does not represent full time interval. Possibly 'TotalMilliseconds' value was intended instead. MSBuild XMake.cs 629

public static ExitType Execute(string commandLine)
{
  ....
  if (!String.IsNullOrEmpty(timerOutputFilename))
  {
      AppendOutputFile(timerOutputFilename, 
                       elapsedTime.Milliseconds);
  }
  ....
}

Pull Request: https://github.com/Microsoft/msbuild/pull/1829

Conclusion

We suggest downloading PVS-Studio analyzer and trying to check your project:

To remove the restrictions of a demo version, you can contact us and we will provide a temporary license key for you.

For a quick introduction to the analyzer, you can use the tools, tracking the runs of the compiler and collect all the necessary information for the analysis. See the description of the utilities CLMonitoring and pvs-studio-analyzer. If you are working with a classic type of project in Visual Studio, everything is much simpler: you should just choose in PVS-Studio menu a command "Check Solution".



Use PVS-Studio to search for bugs in C, C++ and C# code

We offer you to check your project code with PVS-Studio. Just one bug found in the project will show you the benefits of the static code analysis methodology better than a dozen of the articles.

goto PVS-Studio;

Andrey Karpov
Articles: 326


Do you make errors in the code?

Check your code
with PVS-Studio

Static code analysis
for C, C++ and C#

goto PVS-Studio;