WebDavServer icon indicating copy to clipboard operation
WebDavServer copied to clipboard

Be careful with using Path.Combine()

Open MarcusWichelmann opened this issue 7 years ago • 2 comments

The code is using the C# Helper method Path.Combine() in multiple places. For example: https://github.com/FubarDevelopment/WebDavServer/blob/master/src/FubarDev.WebDavServer.FileSystem.DotNet/DotNetDirectory.cs#L101

Do you know that using this method might result in some unwanted behaviours and could cause serious security issues? Like explained in the remarks of the API documentation (https://msdn.microsoft.com/de-de/library/fyy7a5kt%28v=vs.110%29.aspx?f=255&MSPPError=-2147217396) the method call Path.Combine(@"C:\MyWebDavDir", @"C:\Passwords\VerySecretFile.txt") will result in "C:\Passwords\VerySecretFile.txt".

Like in some of the methods in the DotNetFileSystem classes, the second argument of Path.Combine is often user-defined and therefore it might be possible to leave the WebDav root directory and read/write to any file on the system. At least on Windows.

I din't check the full code to make sure this is actually possible, but I'd recommend to replace every call to Path.Combine with one to an own more secure implementation.

And by the way: Many thanks for this awesome library! Kind regards, Marcus Wichelmann

MarcusWichelmann avatar Aug 10 '18 12:08 MarcusWichelmann

That would require one to try to create a file name like C:\Whatever\stuff.txt, which might be possible, because \ isn't recognized as path separator on the ASP.NET Core side.

I have to think about this problem.

EDIT: It seems to me that only Windows systems are affected.

fubar-coder avatar Aug 10 '18 13:08 fubar-coder

Yes, you're right, only Windows should be affected. I just noticed, that new Uri(Uri baseUri, string relativeUri) is also affected. So this might need to be checked, too.

MarcusWichelmann avatar Aug 17 '18 15:08 MarcusWichelmann