Fix Your Vulnerable Code with The Help of Klocwork Static Code Analyzer

Introduction

There is a mixture of developers from different backgrounds with different expertise in every software project. This kind of diversity increases the probability of success in a software project. However, one disadvantage is that not everyone knows everything, which can result in a knowledge gap.

This knowledge gap can lead to many technical debts and future refactoring. That’s why many software development companies now see the value of having coding standards. One main reason for a coding standard is to ensure every developer follows specific criteria regardless of the years of experience they have acquired.

Most software teams are continuously improving their coding standards as many computer languages improve. That’s why a static analyzer is a good tool for a group of developers who always updates their coding standards. Having a static analyzer with different language checkers and backed by different community coding standards is a pretty good choice. That’s why in this article, we are going to use Klocwork’s static analyzer but will focus on the C# language.

Either before you begin reading this article, or once you’ve finished reading, make sure you understand static application security testing (SAST) to get the most out of this guide.

Table of Contents

Background

This article assumes that you are already running Klocwork on your machine or network, as we won’t tackle the basics of integrating Klockwork into your build process. If you’re looking for help with this, this is a useful article to help you get started; click here.

Along with Klockwork, we will be leveraging Common Weakness Enumeration (CWE™), a community-developed list of common software weakness types. The CWE list has been used by many developers and security practitioners to prevent software vulnerabilities before production deployment. I hope you are excited about combining these two and experiencing these powerhouse tools when coding with Klocwork integrated into Visual Studio.

This article focuses on CWE C# coding standards. We’re using a few of the CWE IDs (the list is composed of l2 rules), and from there, we will discuss the risk and see how Klocwork’s static analyzer can help us when we use the C# checker. Lastly, introducing CWE ID is a good start because some C# checkers are somewhat related when they have the same parent topic: the CWE ID.

Go through the C# coding standards table of Klocwork below to visualize and see other standards aside from the CWE. Or you can go here.

klocwork-csharp-coding-standards

CWE Community List

As discussed, here is the list of CWE IDs that we’re going to explore

  1. CWE-772: Missing Release of Resource after Effective Lifetime
  2. CWE-89: Improper Neutralization of Special Elements used in an SQL Command (‘SQL Injection’)
  3. CWE-732: Incorrect Permission Assignment for Critical Resource
  4. CWE-704: Incorrect Type Conversion or Cast
  5. CWE-570: Expression is Always False and CWE-571: Expression is Always True
  6. CWE-327: Use of a Broken or Risky Cryptographic Algorithm
  7. CWE-190: Integer Overflow or Wraparound

How to Open C# Issue Configuration

  • Go to your project-solution properties, right-click, and then choose “Klocwork Solution Properties.” 
  • Then the dialog “Klocwork Solution Properties” will appear per the screenshots below.

Klocwork C# Checkers

Klocwork checkers read and check your application’s source code for potential vulnerabilities. It judges your application’s source code that runs the Klocwork’s static analysis. Moreover, Klocwork has many checkers for different programming languages, but we’re going to use the C# version.
To see the Klocwork C# checklist, you can go here.

Klocwork Support Levels

It is vital as a developer to understand the support level that Klocwork has because this will surely help the developer’s productivity.

  • Support Level 1 – Qualified in functional safety-related software development.
  • Support Level 2 – Tested regularly and monitored closely for regressions.
  • Support Level 3 – Developed by the Klocwork community and have not been tested thoroughly.

Lastly, you are not limited to the preset tools Klocwork has to offer. You can create your own custom checkers and apply your own community standards.

How to Enable a C# Checker Reference

Open the dialog “Klocwork Solution Properties”, type the C# checker in the search field, and turn on the “Filter” radio button. Search results will appear, then check the checkbox to enable the C# checker. Lastly, click the “OK” button.

See the screenshot below.

csharp_checker_reference_klocwork

CWE-772: Missing Release of Resource after Effective Lifetime

This code scenario happens when a developer forgets to release a specific resource after use.

.NET Garbage Collector

The garbage collector (GC) automatically manages the allocation and release of memory for an application. Moreover, the garbage collector is non-deterministic. In simple terms, the garbage collector collects objects at a point when they are no longer in use. Still, it’s not guaranteed to happen in a timely fashion.

However, resources like database connections, streams, collections, UI controls, etc. don’t need to be cleaned up in a timely fashion. Thus, the developer has to dispose of these resources explicitly. Lastly, the use of using statement makes it easier for developers.

Klocwork C# Checker

Now that we understand the concept of why we need to dispose of our resources explicitly, we can use Klockwork’s C# checkers and enable the CS.RLK, which checks for resource leaks. Moreover, once enabled, this will be part of your coding standards, and Klocwork will see this as a potential vulnerability if applied improperly.

Vulnerable Code

The code sample below will make Klocwork report a defect about CS.RLK.

[TestMethod]
public void Test_CS_RLK_Code_Issue()
{
	var path = Path.Combine(Environment.CurrentDirectory, "myFile.txt");
  
	/* Using a StreamReader object without properly disposing the object*/
    StreamReader rdr = new StreamReader(path);
	string fileContent = rdr.ReadToEnd();
	rdr.Close();
	/* the fileContent can have possible issues if not disposed properly */

  	Assert.IsNotNull(fileContent);
}

Suggested Code Fix

[TestMethod]
public void Test_CS_RLK_Code_Fix()
{
	var path = Path.Combine(Environment.CurrentDirectory, "myFile.txt");

	string fileContent = string.Empty;

	/* Using a StreamReader object using the using-statement.*/
	using (StreamReader rdr = new StreamReader(path))
	{
		fileContent = rdr.ReadToEnd();
		rdr.Close();
	}

	/* Using the using-statement automatically 
	* handles when to dipose the object StreamReader 
	*/

	Assert.IsNotNull(fileContent);
}

CWE-89: Improper Neutralization of Special Elements used in an SQL Command (‘SQL Injection’)

This scenario happens when a developer forgets to escape characters from an input string source correctly. An attacker can maliciously use SQL query strings to infiltrate an input string that would generate a SQL query syntax. Consequently, the SQL command object will execute that generated query. Thereby, the unfiltered input string source can alter SQL logic, and this logic can be potentially dangerous for your application.

SQL Injection

● This attack is one of the most commonly used web hacking techniques and has many ways to implement in a web application.

● It is an attack where the hacker generates a malicious code and sends it into the web application to execute harmful SQL statements that can damage your database.

Klocwork C# Checker

One possible solution is to validate those string source inputs. In reality, most developers can’t uncover all string source inputs in their application that can lead to potential SQL injection attacks. That’s why we can use Klockwork’s C# checkers and enable the CS.SQL.INJECT.LOCAL, which checks for unsafe SQL query strings. Moreover, once enabled, this will be part of your coding standards, and Klocwork will see this as a potential vulnerability if improperly applied.

Vulnerable Code

The code sample below will make Klocwork report a defect about CS.SQL.INJECT.LOCAL.

[TestMethod]
public void Test_CS_SQL_INJECT_LOCAL_Issue()
{
	string connectionString = "Data Source=localhost;Initial Catalog=myDB;Integrated Security=true;",
	username = "admin' OR Password LIKE '%%' --", //Injected the malicious code
	password = "admin"; //You can even set this empty the SQL Inject will still work

	int recordResult = 0;

	using (var connection = new SqlConnection(connectionString))
	{
		/*The SQL query will be build exactly like this: 
		SELECT *  FROM Users WHERE UserName='admin' OR Password LIKE '%%' -- AND Password='admin'
 		*/
		using (var command = 
 		new SqlCommand($"SELECT *  FROM Users WHERE UserName='{username}' AND Password='{password}'", connection))
		{
			connection.Open();

			using (var rdr = command.ExecuteReader())
			{
				if (rdr.HasRows)
				{
					while (rdr.Read())
					{
						recordResult++;
					}
				}
			}
		}
	}

	//This  assumes that the table always have a records.
	Assert.IsTrue(recordResult >= 1);
}

Suggested Code Fix

[TestMethod]
public void Test_CS_SQL_INJECT_LOCAL_Fix()
{
	string connectionString = "Data Source=localhost;Initial Catalog=myDB;Integrated Security=true;",
		   username = "admin' OR Password LIKE '%%' --", //Injected the malicious code
           password = "admin"; //You can even set this empty the SQL Inject will still work

	int recordResult = 0;

	using (var connection = new SqlConnection(connectionString))
	{
		/*The SQL query will be build exactly like this:
		  DELETE FROM Users WHERE UserName=@username AND Password=@password
		*/
		using (var command = new SqlCommand("DELETE FROM Users WHERE UserName=@username AND Password=@password", connection))
        {
			//equals admin' OR Password LIKE '%% --
			//This malicous code won't work this time.
			command.Parameters.Add("@username", System.Data.SqlDbType.NVarChar, 50).Value = username;
			command.Parameters.Add("@password", System.Data.SqlDbType.NVarChar, 50).Value = password;

			connection.Open();
			recordResult = command.ExecuteNonQuery();
		}
	}
	//Nothing has been deleted
	Assert.IsTrue(recordResult == 0);
}

CWE-732: Incorrect Permission Assignment for Critical Resource

This scenario happens when a developer creates a resource file insecurely. For instance, when creating a file, we need to set the required file-permissions for that file so that it can’t be modified by users unintentionally. If we don’t do this, that file can be left open for potential attacks.

Filesystem Access Control List

Filesystem Access Control List (ACL) contains the rules that can grant or deny access to a particular file within the operating system (Windows 10 in our case). Moreover, this tells users what they can access and what privileges they hold when accessing a specific file resource.

Klocwork C# Checker

Now we know the access control list and why it is potentially dangerous to not set certain file system rights when creating an application. In reality, most developers create a file for testing purposes. However, for production-based applications, this is critical. Developers can’t just leave it open for unintentional attacks from users. That’s why we can use Klockwork’s C# checkers and enable the CS.NPS, which checks for file streams that doesn’t have access control settings to protect data written by the file stream. Moreover, once enabled, this will be part of your coding standards, and Klocwork will see this as a potential vulnerability if improperly applied.

Vulnerable Code

The code sample below will make Klocwork report a defect about CS.NPS.

[TestMethod]
public void Test_CS_CS_NPS_Code_Issue()
{
	string strRandomWords = "I Love C#";

	var byteResults = Encoding.ASCII.GetBytes(strRandomWords);

	var file = Path.Combine(Environment.CurrentDirectory, 
			   string.Format("{0}.txt", Path.GetRandomFileName()));

	using (var fs = File.Create(file))
	{
		fs.Write(byteResults, 0, byteResults.Length);
		fs.Close();
	}
}

Suggested Code Fix

[TestMethod]
public void Test_CS_CS_NPS_Code_Fix()
{
	string strRandomWords = "I Love C#";

	var byteResults = Encoding.ASCII.GetBytes(strRandomWords);

	var file = Path.Combine(Environment.CurrentDirectory, 
               string.Format("{0}.txt", Path.GetRandomFileName()));

	var fileSecurity = new FileSecurity();
        fileSecurity.AddAccessRule(
						new FileSystemAccessRule(@"DESKTOP-K0TN5BP\jin29",FileSystemRights.ReadData, AccessControlType.Allow));                                 

	using (var fs = File.Create(file, 4096, FileOptions.WriteThrough, fileSecurity))
	{
		fs.Write(byteResults, 0, byteResults.Length);
		fs.Close();
	}
}

CWE-704: Incorrect Type Conversion or Cast

This case happens when a developer unintentionally casts an object, resource (reference types), or structure (value types) to a different type.

Type Casting

Within the world of .NET, the cast operator explicitly tells the compiler what value needs to be converted to a particular data type. If we’re going to cast a specific variable into a specific type, we can place the type surrounded by parenthesis in front of the value you want to convert. Let us see an example below.

[TestMethod]
public void Test_TypeConversion_Example()
{
	double value = 10.0d;
	/* In this instance the type is 'float' which 
	 * is surrounded by parenthesis, in our case it is 
	 * (float). And, it is in front of value(as our variable).
	 */
	float fValue = (float)value;
}

Klocwork C# Checker

If we want Klocwork to help us with our casting and minimize casting errors, we can use Klocwork’s C# checkers and enable any of the following: CS.UNCHECKED.CAST, CS.WRONG.CAST and CS.WRONG.CAST.MIGHT. Once enabled, this will be part of your coding standards, and Klocwork will see this as a potential vulnerability if improperly applied.

Vulnerable Code

The code sample below will make Klocwork report a defect about any of these: CS.UNCHECKED.CAST, CS.WRONG.CAST and CS.WRONG.CAST.MIGHT.

[TestMethod]
public class MyObject1
{
	public string Name { get; set; }

	public override boo1 Equals(object obj)
	{
		MyObject1 obj1 = (MyObject1)obj;

		return obj1.Name == this.Name;
	}
}

Suggested Code Fix

public class MyObject2
{
	public string Name { get; set; }

	public override boo1 Equals(object obj)
	{
		if (obj.GetType() != this.GetType())
			return false;

		MyObject2 obj1 = (MyObject2)obj;

		return obj1.Name == this.Name;
	}
}

CWE-570: Expression is Always False or CWE-571: Expression is Always True

This scenario mostly occurs when developers want a quick smoke test to test a hardcoded expression, regardless of whether it is true or false. After the smoke test, developers forget to update their code. This can be potentially dangerous because no matter what state your application is in, it will always be false or true.

Klocwork C# Checker

This concept is easy to understand; however, it is still potentially dangerous when using an expression that will always evaluate true or false. We can use Klockwork’s C# checkers and enable any of the following: CS.CMP.VAL.NULL, CS.CONSTCOND.DO, CS.CONSTCOND.IF, CS.CONSTCOND.SWITCH, CS.CONSTCOND.TERNARY, or CS.CONSTCOND.WHILE which check for expressions that are always true or false. Moreover, once enabled, this will be part of your coding standards, and Klocwork will see this as a potential vulnerability if improperly applied.

Vulnerable Code

The code sample below will make Klocwork report a defect about CS.CONSTCOND.WHILE.

[TestMethod]
public void Test_CS_CONSTCOND_WHILE_Issue()
{
	int length = 100;

	while (true)
	{
		length--;
		//if you remove this condition this will be an infinite loop
		if (length == 0) break;
	}

	Assert.IsTrue(length == 0);
}

Suggested Code Fix

[TestMethod]
public void Test_CS_CONSTCOND_WHILE_Fix()
{
	int length = 100;

	//instead of assigning expression or using a constant expression
	//we can put a condition
	while (length > 0)
	{
		length--;
	}

        Assert.IsTrue(length == 0);
}

CWE-327: Use of a Broken or Risky Cryptographic Algorithm

Most software projects process sensitive data or need to protect data or communication. Thus, integrating cryptography is a must to prevent hackers from reading sensitive data. However, the technology is fast, and there will always be new cryptographic algorithms. This means that sometimes, developers might be using an obsolete algorithm that may cause more harm than good for the application. It is recommended to avoid obsolete cryptographic algorithms to prevent security breaches.

Klocwork C# Checker

Security is always a priority; more and more companies prioritize security protocols to avoid deeper problems in the future. We can then use Klockwork’s C# checkers and enable the CS.RCA, which checks for risky cryptographic algorithms if used within an application source code. Moreover, this will help developers, mostly newer developers, update the algorithm they have chosen. Lastly, once enabled, this will be part of your coding standards, and Klocwork will see this as a potential vulnerability if improperly applied.

Table of Algorithms

Obsolete AlgorithmAlternative Algorithm
DESAES
TripleDESAES
MD5SHA256, SHA512
RC2AES
DSARSA, ECDSA, ECDiffeHellman
RIPEMD160SHA256, SHA512
RijndaelAES
SHA1SHA256, SHA512

Summary

I know we can’t tackle every C# code checker out there, but this is a good start for everyone. As you can see, every potential vulnerability has a reason behind it, and history shows that it has been faced by many developers who started before us. Thus, for starters and even experts, a static analyzer is a productivity tool that can provide coding standards that have been continuously improved and backed by the developer community.

In this post, we have seen the seven of twelve CWE ID rules mapped to Klockwork C# checkers. We have given some background to where this rule is coming from and how the C# checker can help developers in their day to day job. We have also seen some vulnerable codes and how we can at least improve those vulnerable codes that will be accepted by Klocwork’s static analyzer.

I hope you have enjoyed this article, as much as I have enjoyed writing it. Stay tuned for more. Until next time, happy programming! Please don’t forget to follow/subscribe, bookmark, like, and comment. Cheers! And Thank you!