Skip to content

Commit

Permalink
Fix security issues with filesystems, 2nd attempt (#70)
Browse files Browse the repository at this point in the history
  • Loading branch information
xoofx committed Nov 14, 2022
1 parent 13e1ee1 commit 930974d
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 6 deletions.
4 changes: 4 additions & 0 deletions src/Zio.Tests/FileSystems/TestPhysicalFileSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ public void TestDirectorySpecial()
public void TestDirectoryExceptions()
{
var fs = new PhysicalFileSystem();

// Test invalid characters in path
Assert.Throws<ArgumentException>(() => fs.CreateFile($"/##/mnt/hello".Replace("#", char.ConvertFromUtf32(0xad))));

// Try to create a folder on an unauthorized location
fs.CreateDirectory("/");
Assert.Throws<UnauthorizedAccessException>(() => fs.CreateDirectory("/mnt2"));
Expand Down
4 changes: 2 additions & 2 deletions src/Zio/FileSystems/PhysicalFileSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,7 @@ protected override string ConvertPathToInternalImpl(UPath path)

if (IsOnWindows)
{
if (!absolutePath.StartsWith(DrivePrefixOnWindows) ||
if (!absolutePath.StartsWith(DrivePrefixOnWindows, StringComparison.Ordinal) ||
absolutePath.Length == DrivePrefixOnWindows.Length ||
!IsDriveLetter(absolutePath[DrivePrefixOnWindows.Length]))
throw new ArgumentException($"A path on Windows must start by `{DrivePrefixOnWindows}` followed by the drive letter");
Expand Down Expand Up @@ -829,7 +829,7 @@ protected override UPath ConvertPathFromInternalImpl(string innerPath)
if (IsOnWindows)
{
// We currently don't support special Windows files (\\.\ \??\ DosDevices...etc.)
if (innerPath.StartsWith(@"\\") || innerPath.StartsWith(@"\?"))
if (innerPath.StartsWith(@"\\", StringComparison.Ordinal) || innerPath.StartsWith(@"\?", StringComparison.Ordinal))
throw new NotSupportedException($"Path starting with `\\\\` or `\\?` are not supported -> `{innerPath}` ");

var absolutePath = Path.GetFullPath(innerPath);
Expand Down
2 changes: 1 addition & 1 deletion src/Zio/FileSystems/SubFileSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ protected override UPath ConvertPathToDelegate(UPath path)
protected override UPath ConvertPathFromDelegate(UPath path)
{
var fullPath = path.FullName;
if (!fullPath.StartsWith(SubPath.FullName) || (fullPath.Length > SubPath.FullName.Length && fullPath[SubPath.FullName.Length] != UPath.DirectorySeparator))
if (!fullPath.StartsWith(SubPath.FullName, StringComparison.Ordinal) || (fullPath.Length > SubPath.FullName.Length && fullPath[SubPath.FullName.Length] != UPath.DirectorySeparator))
{
// More a safe guard, as it should never happen, but if a delegate filesystem doesn't respect its root path
// we are throwing an exception here
Expand Down
2 changes: 1 addition & 1 deletion src/Zio/SearchPattern.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ private SearchPattern(ref UPath path, ref string searchPattern)
return;
}

if (searchPattern.StartsWith("/"))
if (searchPattern.StartsWith("/", StringComparison.Ordinal))
{
throw new ArgumentException($"The search pattern `{searchPattern}` cannot start by an absolute path `/`");
}
Expand Down
2 changes: 1 addition & 1 deletion src/Zio/UPath.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ internal UPath(string path, bool safe)
/// Gets a value indicating whether this path is absolute by starting with a leading `/`.
/// </summary>
/// <value><c>true</c> if this path is absolute; otherwise, <c>false</c>.</value>
public bool IsAbsolute => FullName?.StartsWith("/") ?? false;
public bool IsAbsolute => FullName?.StartsWith("/", StringComparison.Ordinal) ?? false;

/// <summary>
/// Gets a value indicating whether this path is relative by **not** starting with a leading `/`.
Expand Down
2 changes: 1 addition & 1 deletion src/Zio/UPathExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ public static bool IsInDirectory(this UPath path, UPath directory, bool recursiv
var target = path.FullName;
var dir = directory.FullName;

if (target.Length < dir.Length || !target.StartsWith(dir))
if (target.Length < dir.Length || !target.StartsWith(dir, StringComparison.Ordinal))
{
return false;
}
Expand Down

0 comments on commit 930974d

Please sign in to comment.