An anonymous submitter from Hungary reached out with both some bad code, and a story behind it.

Hungary's school system runs on a software package called KRÉTA. The Sawarim$ hacker group felt that the software was badly designed and left millions of students personal information unprotected- so they hacked in to prove it. The company running the software responded in the worst possible way- by attempting to cover up the breach and pretending nothing ever happened. It's quite the news story.

The hacking group, not interested in releasing any students' private information, instead released the C# source code. Our Anonymous submitter reviewed some of that code, and sends us one method from it.

/// <summary>
/// Removes all elements that could cause problems with input fields
/// To be used with with LoadWithFilter
/// </summary>
/// <param name="text"></param>
/// <returns></returns>
public static string PreventSQLInjection(string dirtytext)
{
    List<string> disallowedtags = new List<string>()
    {
        "'", " or ", " OR ", " and ", " AND ",
        "=", "<", ".", "<>", " not ", " NOT "
    };
    //theoretically because I trim at first space there can be no AND, OR, or NOT left
    string cleartext = dirtytext;
    
    if (cleartext.Contains(" "))
    {
        cleartext = cleartext.Substring(0, cleartext.IndexOf(" ") + 1).Trim();
    }
    
    foreach (string tag in disallowedtags)
    {
        cleartext = cleartext.Replace(tag, "");
    }
    
    return cleartext;
}

With a method called PreventSQLInjection, I can't imagine how the hackers got in.

I suspect that all of the comment is nonsense- the return value and the parameter are incorrectly documented, I wouldn't think that the summary is correct either. And the very existence of this method tells us that there's something wrong- they're almost certainly building SQL queries via string concatenation.

But, let's trace through this garbage. We start by defining a set of disallowedTags. Which, you'll note, are done in both lower and upper case, because string comparisons are case sensitive by default.

Then, we check if the text includes a space. If it does, we substring the entire string up to that space, including the space, then Trim the space off. This gives us a cleartext value which contains no spaces. Then we iterate through our disallowedtags, Replaceing each substring of our spaceless cleartext with an empty string. Which, also means many of our disallowedtags are never replaced, as they contain spaces. This is something the developer appears to be aware of, at least "theoretically".

"Replacing invalid strings" is always a bad way to prevent SQL injection attacks, because it means you're building queries through string concatenation instead. But this is an exceptionally bad version of this.

Now, our anonymous submitter isn't the sort of person who downloads code posted by hacker groups on Telegram, which is probably for the best. They found this in a discussion of the source. So our anonymous submitter adds:

The best part? According to someone who claims to have downloaded the full source code, this function is never called.

Surely, they must have a better way to protect against SQL injections that they're calling instead. Right? Right?

<narrator>They don't</narrator>