Trim().Split causes a problem in Contains()

Viewed 324

I am getting a string and trimming it first, then splitting it and assigning it to a string[]. Then, I am using every element in the array for a string.Contains() or string.StartsWith() method. Interesting thing is that even if the string contains element, Contains() doesn't work properly. And situation is same for StartsWith(), too. Does anyone have any idea about the problem?

P.S.: I trimmed strings after splitting and problem was solved.

string inputTxt = "tasklist";
string commands = "net, netsh, tasklist";

string[] maliciousConsoleCommands = commands.Trim(' ').Split(',');

for (int i = 0; i < maliciousConsoleCommands.Length; i++) {
    if (inputTxt.StartsWith(maliciousConsoleCommands[i])) {
        return false;
    }
} 

//this code works but no idea why previous code didn't work.

string[] maliciousConsoleCommands = commands.Split(',');

for (int i = 0; i < maliciousConsoleCommands.Length; i++) {
    if (inputTxt.StartsWith(maliciousConsoleCommands[i].Trim(' '))) {
        return false;
    }
}

I expected to work properly but it is solved by trimming after splitting.

4 Answers

Your delimiter is not a comma char, it's a comma followed by a white-space - so instead of splitting by ',', simply split by ", ":

string[] maliciousConsoleCommands = commands.Split(new string[] {", "});

This will return the items without the leading space so the trim will be redundant.

It seems, you should Trim each item :

// ["net", "netsh, "tasklist"]
string[] maliciousConsoleCommands = commands
  .Split(',')                   // "net" " netsh", " tasklist" - note leading spaces
  .Select(item => item.Trim())  // removing leading spaces from each item
  .ToArray();

Finally, if you want to test if inputTxt is malicious:

if (commands
      .Split(',')
      .Select(item => item.Trim()) // You can combine Select and Any            
      .Any(item => inputTxt.StartsWith(item))
  return false;

First code you presented won't work because you want to trim initial string, so "net, netsh, tasklist" will stay unchanged after trimming (no leading and trailing spaces), then splitting it by comma will produce entries, that have leading space. Thus, you will get unexpected results. You should be trimming after splitting the string.

Second code also won't work, because you use Trim after StartsWith, which return bool value. You can't apply Trim to bool, this code should not even compile.

Yet another way to split if the commands themselves have no spaces is to use ' ' itself as a delimiter, and discard empty entries :

var  maliciousConsoleCommands =  commands.Split(new[]{',',' '},StringSplitOptions.RemoveEmptyEntries)
                                         .ToArray();

This avoids the temporary strings generated by every string manipulation command.

For your code to work though, you'd have use Contains for each command, instead of using StartWith :

var isSuspicious = maliciousCommands.Any(cmd=>input.Contains(cmd));

Or even :

var isSuspicious = maliciousCommands.Any(input.Contains);

This can get rather slow if you have multiple commands, or if the input text is large

Regular expression alternative

A far faster technique would be to use a Regular expression. This performs a lot faster than searching individual keywords :

var regex=new Regex("net|netsh|tasklist");
var isSuspicious=regex.IsMatch(inputTxt);

Regular expressions are thread-safe which means they can be created once and reused by different threads/requests.

By using Match/Matches instead of IsMatch the regex could return the actual keywords that were detected :

var detection=regex.Match(inputTxt);
if (detection.Success)
{
    var detectedKeyword=detection.Value;
    ....
}

Converting the original comma-separated list to a regular expression can be performed with a single String.Replace(", ") or another regular expression that can handle any whitespace character :

string commands = "net , netsh, \ttasklist";
var pattern=Regex.Replace(commands,@"\s*,\s*","|").Dump();
var regex=new Regex(pattern);

Detecting whole words only

Both Contains and the original regular expression would match tasklist1 as well as tasklist. It's possible to match whole words only, if the pattern is surrounded by the word delimiter, \b :

@"\b(" + pattern + @")\b"

This will match tasklist and net but reject tasklist1

Related