Fortify shows critical vulnerability File.Delete() operation C#
The following code always shows path manipulation problem. How to resolve it ?
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.IO;
using System.Text.Regula开发者_C百科rExpressions;
namespace PathManipulation
{
class Program
{
public string dir = null;
public void someFunction(string fileName)
{
// File.Delete(Regex.Replace(dir + fileName, @"\..\", String.Empty));
if (!(dir.IndexOf("//") >= 0) || !Regex.IsMatch(dir, "System32"))
{
String p = Regex.Replace(dir, @"..\", string.Empty);
DirectoryInfo di = new DirectoryInfo(p);
FileInfo[] fi = di.GetFiles();
if (fi.Length > 0)
{
for (int i = 0; i < fi.Length; i++)
{
if (fi[i].ToString().Equals(fileName))
{
Console.WriteLine(fi[i].ToString());
fi[i].Delete();
}
}
File.Delete(dir + fileName);
}
}
else
{
return;
}
}
static void Main(string[] args)
{
Program p = new Program();
p.dir = args[0];
p.someFunction(args[1]);
}
}
}
Yes, you break the flow of data so that the end user is not able to specify the file to be deleted.
For instance:
public void someFunction(int fileIndex){
...
if (fileIndex == 0){
File.Delete( "puppies.txt" );
}
else if (fileIndex == 1){
File.Delete( "kittens.txt" );
}
else {
throw new IllegalArgumentException( "Invalid delete index" );
}
}
That's an extreme way to solve the problem, but it does not allow the end user to delete ANYTHING the developer didn't intend.
Your data validation check:
if (!(dir.IndexOf("//") >= 0) || !Regex.IsMatch(dir, "System32"))
is weak. This is referred to as "blacklisting" and the attacker only has to figure out a pattern that is missed by your checks. So, @"C:\My Documents" for instance.
Instead, you should consider a "whitelisting" approach. Take a look at https://www.owasp.org/index.php/Data_Validation#Accept_known_good for a pretty thorough example. It doesn't address path injection directly. You simply have to think hard about what files/directories you expect to receive. Throw an error if the input deviates from that. With a little testing you will create a good whitelist.
精彩评论