1

I have a Rest Service which is inputs as parameters.Inside that there is a Path.Combine method which is used to generate a path.But in veracode it catch Path.Combine method for Directory Traversal Injection. Any possible ways to fix the issue.

var path = HttpContext.Current.Server.MapPath("~/MainFolder");
var name ="sampleLog";
var filename = String.Format("{0}.txt",name);

var fullpath = Path.Combine(path, filename); // Veracode shows this method as a possible injection

I have tried to validate the filename using the following method , but it did n't take as a fix.

private string CleanFileName(string name)
{
   return Path.GetInvalidFileNameChars().Aggregate(name, (current, c) => current.Replace(c.ToString(), string.Empty));
}

Any other possible solution to avoid this fix this issue ?

Damien_The_Unbeliever
  • 234,701
  • 27
  • 340
  • 448
LahiruD
  • 93
  • 1
  • 12
  • You should check that after combining path and filename, resulting path still points to the directory `path` and not to some other point in your filesystem (by checking `Directory.GetDirectoryName(Path.GetFullPath(combined))` for example). – Evk Oct 27 '17 at 07:45
  • Besides, what issue? The application pool account has *no* permissions to access anything outside the site's folder. One would have to do quite some work to bypass this, eg explicitly give permission to the entire drive to the pool account, or use a real user account with wide permissions. – Panagiotis Kanavos Oct 27 '17 at 07:49
  • In fact, if you search SO you'll find a lot of questions asking why they *can't* access folders outside the path. You *should* do what Evk says and check the path, to avoid getting an AccessDenied exception. Validating the input would be a good idea, to find out about script kiddie attacks. – Panagiotis Kanavos Oct 27 '17 at 07:52
  • Besides, exceptions cost in CPU terms and therefore cloud/hosting money. You don't want to pay for a script kiddie's amusement. *Banning an IP* that sends such names would be a nice idea. – Panagiotis Kanavos Oct 27 '17 at 07:54
  • 1
    @PanagiotisKanavos - it appears that they're using some tool ([tag:veracode]) that is flagging this as potentially unsafe. As you say, sound permissions/config should make it a non-issue but apparently the tool encourages a belt-and-braces approach here. – Damien_The_Unbeliever Oct 27 '17 at 08:29
  • @Damien_The_Unbeliever I call it the `protect my pocketbook` approach. Check the input to avoid the exception, log the attempt and block the IP if there are repeated calls. Finding out you are paying the equivalent one server in the farm just for kiddie attacks is very annoying – Panagiotis Kanavos Oct 27 '17 at 08:34
  • @Damien_The_Unbeliever unfortunately, a tool like veracode may not understand what the code does, just detect text patterns. It may not understand that the path is validated. I suspect one could use a FileInfo object to create a *vulnerable* path but the tool won't detect it – Panagiotis Kanavos Oct 27 '17 at 08:36
  • It's unclear why this is four lines of code rather than just `.MapPath("~/MainFolder/sampleLog.txt");`. I'm guessing that this isn't the real code and that some inputs are coming from the request? – Damien_The_Unbeliever Oct 27 '17 at 08:36
  • @Damien_The_Unbeliever the name may be an action parameter, in which case this *may* be vulnerable. In self-hosted ASP.NET Core code, this *could* lead to directory traversal if the account isn't restricted. The problem is that the tool doesn't recognize the validation – Panagiotis Kanavos Oct 27 '17 at 08:55
  • Possible fix and a very good idea: [Don't accept file names, use IDs or other indicators](https://www.filipekberg.se/2013/07/12/are-you-serving-files-insecurely-in-asp-net/) and map them to the actual file. – Panagiotis Kanavos Oct 27 '17 at 08:56
  • Possible fix that *may* not be as good: Use `DirectoryInfo.EnumerateFiles()` to return and use a FileInfo, not just a path. GetFiles/EnumerateFiles only return files inside the folder, eg `var dir=new DirectoryInfo(path); var fi=dir.EnumerateFiles(name).First(); ...` – Panagiotis Kanavos Oct 27 '17 at 08:59
  • @Damien_The_Unbeliever Yes it is getting input parameters. I added only the directory traversal issue occurring code snippet – LahiruD Oct 27 '17 at 09:41

0 Answers0