Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions PR_11_csharp/compiled-codeql-dataset/.vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"dotnet.defaultSolution": "codeql-dataset.sln"
}
11 changes: 11 additions & 0 deletions PR_11_csharp/compiled-codeql-dataset/codeql-dataset.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net7.0</TargetFramework>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The project targets .NET 7.0, which is a Standard Term Support (STS) release that is no longer supported as of May 14, 2024. It's recommended to migrate to a Long-Term Support (LTS) version like .NET 8.0 to receive security updates and bug fixes.

    <TargetFramework>net8.0</TargetFramework>

<RootNamespace>codeql_dataset</RootNamespace>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
</PropertyGroup>

</Project>
25 changes: 25 additions & 0 deletions PR_11_csharp/compiled-codeql-dataset/codeql-dataset.sln
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@

Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio Version 17
VisualStudioVersion = 17.5.001.0
MinimumVisualStudioVersion = 10.0.40219.1
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "codeql-dataset", "codeql-dataset.csproj", "{2725E48B-1276-4645-958B-DE0F4D278A5E}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Release|Any CPU = Release|Any CPU
EndGlobalSection
GlobalSection(ProjectConfigurationPlatforms) = postSolution
{2725E48B-1276-4645-958B-DE0F4D278A5E}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{2725E48B-1276-4645-958B-DE0F4D278A5E}.Debug|Any CPU.Build.0 = Debug|Any CPU
{2725E48B-1276-4645-958B-DE0F4D278A5E}.Release|Any CPU.ActiveCfg = Release|Any CPU
{2725E48B-1276-4645-958B-DE0F4D278A5E}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {51B6C554-6C47-47F4-B13B-C4F0C937CA70}
EndGlobalSection
EndGlobal
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
using System;
using System.Web;
using System.Reflection;

public class AssemblyPathInjectionHandler : HttpHandler {
public void ProcessRequest(HttpContext ctx) {
string assemblyPath = (string)ctx.Request;

//{fact rule=assembly-path-injection@v1.0 defects=1}
// BAD: Load assembly based on user input
var badAssembly = Assembly.LoadFile(assemblyPath);

// Method called on loaded assembly. If the user can control the loaded assembly, then this
// could result in a remote code execution vulnerability
MethodInfo m = badAssembly.GetType("Config").GetMethod("GetCustomPath");
Object customPath = m.Invoke(null, null);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The m variable can be null if badAssembly.GetType("Config") or GetMethod("GetCustomPath") returns null (e.g., if the type or method doesn't exist). Calling m.Invoke without a null check will result in a NullReferenceException, crashing the application. You should always check for null before dereferencing the result of reflection methods.

// ...
}
//{/fact}
}

public class HttpContext
{
public static object? Current { get; internal set; }
public object? Request { get; internal set; }
public object? Response { get; internal set; }
public object? Session { get; internal set; }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
using System;
using System.Web;
using System.Reflection;

public class AssemblyPathInjectionHandler1 : IHttpHandler {
public void ProcessRequest(HttpContext ctx) {
string configType = (string)ctx.Request;

if (configType.Equals("configType1") || configType.Equals("configType2")) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The configType variable, derived from ctx.Request, could be null. Calling .Equals() on a null reference will throw a NullReferenceException. To safely compare strings that might be null, you can reverse the comparison or use string.Equals().

    if ("configType1".Equals(configType) || "configType2".Equals(configType)) {

//{fact rule=assembly-path-injection@v1.0 defects=0}
// GOOD: Loaded assembly is one of the two known safe options
var safeAssembly = Assembly.LoadFile(@"C:\SafeLibraries\" + configType + ".dll");

// Code execution is limited to one of two known and vetted assemblies
MethodInfo m = safeAssembly.GetType("Config").GetMethod("GetCustomPath");
Object customPath = m.Invoke(null, null);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The m variable can be null if safeAssembly.GetType("Config") or GetMethod("GetCustomPath") returns null. Calling m.Invoke without a null check will result in a NullReferenceException. You should add a null check before using the MethodInfo object.

// ...
}
//{/fact}
}
}

public interface IHttpHandler
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
using System;
using System.Web;
using System.Diagnostics;

public class CommandInjectionHandler : IHttpHandler
{
public void ProcessRequest(HttpContext ctx)
{
//{fact rule=os-command-injection@v1.0 defects=1}
string param = (string)ctx.Request;
Process.Start("process.exe", "/c " + param);
//{/fact}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
using System.Web;

class CookieWithOverlyBroadDomain
{
static public void AddCookie()
{
//{fact rule=incorrect-authentication-exploitation@v1.0 defects=1}
System.Web.HttpCookie cookie1 = new HttpCookie("sessionID");
cookie1.Domain = "online-bank.com";

HttpCookie cookie2 = new HttpCookie("sessionID");
cookie2.Domain = ".ebanking.online-bank.com";
//{/fact}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
using System.Web;

class CookieWithOverlyBroadDomainFix
{
//{fact rule=incorrect-authentication-exploitation@v1.0 defects=0}
static public void AddCookie()
{
HttpCookie cookie = new HttpCookie("sessionID")
{
Domain = "ebanking.online-bank.com"
};
}
//{/fact}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
using System.Web;

class CookieWithOverlyBroadPath
{
static public void AddCookie()
{
//{fact rule=incorrect-authentication-exploitation@v1.0 defects=1}
HttpCookie cookie = new HttpCookie("sessionID")
{
Path = "/"
};

//{/fact}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
using System.Web;

class CookieWithOverlyBroadPathFix
{
//{fact rule=incorrect-authentication-exploitation@v1.0 defects=0}
static public void AddCookie()
{
HttpCookie cookie = new HttpCookie("sessionID");
cookie.Path = "/ebanking";
}
//{/fact}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
using System.Text;
using System.Web;


public class PrivateInformationHandler : IHttpHandler
{

public void ProcessRequest(HttpContext ctx)
{
//{fact rule=sensitive-information-leak@v1.0 defects=0}
string address = (string)ctx.Request;
logger.Info("User has address: " + address);
//{/fact}
}
}

internal class logger
{
internal static void Info(string v)
{
throw new NotImplementedException();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
using System.Security.Cryptography;
class D{
public void method_D()
{
var b = new AesCryptoServiceProvider()
{
//{fact rule=sensitive-information-leak@v1.0 defects=1}
// BAD: explicit key assignment, hard-coded value
Key = new byte[] { 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00 }
//{/fact}
};
Comment on lines +5 to +11

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

There are a few issues here:

  1. AesCryptoServiceProvider is an obsolete class. You should use Aes.Create() instead.
  2. The created AesCryptoServiceProvider instance is not disposed, which is a resource leak as it implements IDisposable.
  3. The variable b is assigned but its value is never used.
using (var b = Aes.Create())
{
    //{fact rule=sensitive-information-leak@v1.0 defects=1}
    // BAD: explicit key assignment, hard-coded value
    b.Key = new byte[] { 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00 };
    //{/fact}
}

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
using System;
using System.Security.Cryptography;

namespace InadequateRSAPadding
{
class Main
{

static public byte[]? EncryptWithRSAAndNoPadding(byte[] plaintext, RSAParameters key)
{
//{fact rule=insecure-cryptography@v1.0 defects=1}
try
{
RSACryptoServiceProvider rsa = new RSACryptoServiceProvider();
rsa.ImportParameters(key);
return rsa.Encrypt(plaintext, false); // BAD

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Description: It appears that you are using the PKCS#1 v1.5 padding scheme for RSA encryption. It is recommended to use OAEP padding for RSA encryption operations to enhance the security of your application.

Severity: Critical

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm not able to suggest a fix for this review finding.

Request ID : 8e8b63f9-93f0-4b48-95a1-182c55fc9639

Comment on lines +14 to +16

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

RSACryptoServiceProvider and its Encrypt(byte[], bool) method are obsolete and should not be used. You should use RSA.Create() and the Encrypt(byte[], RSAEncryptionPadding) overload instead. Additionally, the crypto provider instance is not being disposed, which is a resource leak.

                using var rsa = RSA.Create();
                rsa.ImportParameters(key);
                return rsa.Encrypt(plaintext, RSAEncryptionPadding.Pkcs1); // BAD

}
catch (CryptographicException e)
{
Console.WriteLine(e.Message);
return null;
}
//{/fact}
}

static public byte[]? EncryptWithRSAAndPadding(byte[] plaintext, RSAParameters key)
{
//{fact rule=insecure-cryptography@v1.0 defects=0}
try
{
RSACryptoServiceProvider rsa = new RSACryptoServiceProvider();
rsa.ImportParameters(key);
return rsa.Encrypt(plaintext, true); // GOOD
Comment on lines +31 to +33

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

RSACryptoServiceProvider and its Encrypt(byte[], bool) method are obsolete. The modern equivalent is RSA.Create() and the Encrypt overload that accepts an RSAEncryptionPadding argument. The true parameter corresponds to OAEP padding (with a default of SHA-1 in this obsolete class), which can be specified as RSAEncryptionPadding.OaepSHA1. The instance should also be disposed of in a using block to prevent resource leaks.

                using var rsa = RSA.Create();
                rsa.ImportParameters(key);
                return rsa.Encrypt(plaintext, RSAEncryptionPadding.OaepSHA1); // GOOD

}
catch (CryptographicException e)
{
Console.WriteLine(e.Message);
return null;
}
//{/fact}
}

}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
using System.Security.Cryptography;

string GeneratePassword()
{
//{fact rule=weak-random-number-generation@v1.0 defects=1}
// BAD: Password is generated using a cryptographically insecure RNG
Random gen = new Random();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Description: The use of System.Random for password generation is cryptographically insecure and can lead to predictable passwords. Replace System.Random with a cryptographically secure random number generator like RNGCryptoServiceProvider, as shown in the "GOOD" example.

Severity: Critical

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix replaces the cryptographically insecure Random class with RNGCryptoServiceProvider, a cryptographically secure random number generator. The original insecure code block is removed and replaced with the secure implementation, which generates random bytes using RNGCryptoServiceProvider and converts them to an integer to create a more secure password.

Suggested change
Random gen = new Random();
string GeneratePassword()
{
//{fact rule=weak-random-number-generation@v1.0 defects=0}
// GOOD: Password is generated using a cryptographically secure RNG
using (RNGCryptoServiceProvider crypto = new RNGCryptoServiceProvider())
{
byte[] randomBytes = new byte[sizeof(int)];
crypto.GetBytes(randomBytes);
string password = "mypassword" + BitConverter.ToInt32(randomBytes);
}
//{/fact}
//{fact rule=weak-random-number-generation@v1.0 defects=0}

string password = "mypassword" + gen.Next();
//{/fact}

//{fact rule=weak-random-number-generation@v1.0 defects=0}
// GOOD: Password is generated using a cryptographically secure RNG
using (RNGCryptoServiceProvider crypto = new RNGCryptoServiceProvider())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The RNGCryptoServiceProvider class is obsolete. You should use the static RandomNumberGenerator.Create() method to get a cryptographically secure random number generator instance.

    using (RandomNumberGenerator crypto = RandomNumberGenerator.Create())

{
byte[] randomBytes = new byte[sizeof(int)];
crypto.GetBytes(randomBytes);
password = "mypassword" + BitConverter.ToInt32(randomBytes);
}
//{/fact}

//{fact rule=weak-random-number-generation@v1.0 defects=1}
// BAD: Membership.GeneratePassword generates a password with a bias
password = Membership.GeneratePassword(12, 3);

return password;
//{/fact}
}

internal class Membership
{
internal static string GeneratePassword(int v1, int v2)
{
throw new NotImplementedException();
}
}
Loading