Security analysis of a repository pattern and asking ChatGPT about it.

This week I discovered that SAST tools are struggling with a repository pattern.

So, let's say we have an IRepo interface with a single defined method.

We have 2 implementations: Repo and DummyRepo

    public interface IRepo
    {
        void DoRepoStuff(string username, string name);
    }

    public class Repo : IRepo
    {
        public void DoRepoStuff(string username, string name)
        {
            string concatSql = "SELECT * FROM Accounts WHERE Username='" + username + "' OR  Name='" + name + "'";

            using (SqlConnection connection = new SqlConnection("dummyconnectionstring"))
            {
                SqlCommand concatSqlCommand = new SqlCommand()
                {
                    CommandText = concatSql,
                    CommandType = CommandType.Text,
                };

                concatSqlCommand.ExecuteReader();
            }
        }
    }

    public class DummyRepo : IRepo
    {
        public void DoRepoStuff(string username, string name)
        {
            return;
        }
    }

We declare instances somewhere.

public IRepo _repoVulnerable = new Repo();

public IRepo _repoDummy = new DummyRepo();

Test case 1: Let's call a vulnerable method DoRepoStuff of _repoVulnerable instance. Note, that it is IRepo.

public IActionResult SqlClientVulnerable(string username, string name)
    {
        _repoVulnerable.DoRepoStuff(username, name);
        return new OkResult();
    }

So we found a security vulnerability - SQL Injection, right?
Yes, but not all tools will report it.

Test case 2: Let's call a safe method DoRepoStuff of _repoDummy instance. Note, that it is IRepo.

public IActionResult SqlClientDummySafe(string username, string name)
{
    _repoDummy.DoRepoStuff(username, name);
    return new OkResult();
}

We should not have a problem here, but some security tools will report security vulnerability by following the IRepo interface.

Test case 3:
Declare _repoVulnerable as Repo and _repoDummy as DummyRepo:

public Repo _repoVulnerable = new Repo();

public DummyRepo _repoDummy = new DummyRepo();

It can help some tools to properly identify the true positive _repoVulnerable and do not trigger safe _repoDummy because no need to do a taint analysis by interface.

I can try to group security tools by behaviour.

1. Following interface implementations and marking all invocations vulnerable
   1.1. If a tool displays only a single source, you may not see this behaviour
   1.2. If a tool displays all sources that lead to the sink, you will most likely get a security vulnerability for the Dummy repo.

2. Not following interface
   2.1. Do not display anything - no findings
   2.2. Do not use Taint Analysis decreasing the confidence level.

Which tools are doing what.

1. Github Code Scanning will follow by interface, thus you will identify all true positives along with dummy false positives. Dummy interface implementation simply returns return containing no security issues.

1. SonarCloud will follow by interface but it shows a single source for a single sink. So a single finding.
   Note that SonarCloud has 2 rules to detect SQL injections:
   2.1. S2077 doesn't use Taint Analysis, thus it is a security hotspot.
   2.2. S3649 use Taint Analysis, but like I mention it doesn't show all paths.

2. Security Code Scan is not following by interface, at least in NoAudit mode.

3. Semgrep has no clue about used sinks and shows nothing. Knowing its taint analysis limitation, it should find a known sink in the best case without providing the code flow.

4. Snyk Code use a single source to report a vulnerability, so it is hard to tell for sure if the Dummy stuff is in equation. It also decreases security severity of the finding to Low in my case.

Which is funny, because the same issue is reported as High without using interfaces. So, it is more about confidence then?

Summary
The repository pattern can be tricky for static code analyzers, be ready to do some manual review!

PS: I mentioned ChatGPT, so let's ask it.
Why static code analyzers cannot properly check code that uses a repository pattern?

Answer:

Static code analyzers are tools that automatically scan source code to identify potential issues, such as bugs, security vulnerabilities, and coding errors. These tools are typically designed to work with a specific programming language and follow a set of rules and best practices for that language.

One of the challenges of using static code analyzers with code that uses a repository pattern is that the repository pattern is a design pattern that is implemented in a variety of ways, and each implementation can have its own unique characteristics. Because static code analyzers are designed to work with specific programming languages and follow a set of rules, they may not be able to properly analyze code that uses a repository pattern if it deviates from those rules.

Additionally, the repository pattern is often used to abstract away the details of how data is stored and retrieved, which can make it difficult for static code analyzers to understand the full context of the code and identify potential issues. For these reasons, static code analyzers may have difficulty properly checking code that uses a repository pattern.

Sounds smart but not a real answer. I consider it as a nice try!

My code repository is located here.