Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions SecureCoding.Test/SecureCoding.Test.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
</PackageReference>
<PackageReference Include="FluentAssertions" Version="6.12.0" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.13.0" />
<PackageReference Include="MSTest.TestAdapter" Version="3.8.0" />
<PackageReference Include="MSTest.TestFramework" Version="3.8.0" />
<PackageReference Include="MSTest.TestAdapter" Version="3.8.2" />
<PackageReference Include="MSTest.TestFramework" Version="3.8.2" />
<PackageReference Include="Newtonsoft.Json" Version="13.0.3" />
</ItemGroup>

Expand Down
97 changes: 88 additions & 9 deletions SecureCoding.Test/SecureIO/SecurePathTests.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
namespace SecureCoding.Test.SecureIO
{
using System;
using System.Linq;
using FluentAssertions;
using Microsoft.VisualStudio.TestTools.UnitTesting;

using Skyline.DataMiner.Utils.SecureCoding.SecureIO;

[TestClass]
Expand All @@ -17,7 +15,7 @@ public class SecurePathUnitTests
// network paths
[DataRow(@"\\127.0.0.1\c$\skyline dataminer\", @"test.txt", @"\\127.0.0.1\c$\skyline dataminer\test.txt")]
[DataRow(@"\\127.0.0.1\c$\skyline dataminer", @"test.txt", @"\\127.0.0.1\c$\skyline dataminer\test.txt")]
public void ConstructSecurePathSuccess(string basePath, string filename, string expectedResult)
public void ConstructSecureFilePathSuccess(string basePath, string filename, string expectedResult)
{
string result = SecurePath.ConstructSecurePath(basePath, filename);
Assert.AreEqual(result, expectedResult);
Expand All @@ -30,7 +28,7 @@ public void ConstructSecurePathSuccess(string basePath, string filename, string
// network paths
[DataRow(@"\\127.0.0.1\c$\skyline dataminer\", @"subdir\test.txt", @"\\127.0.0.1\c$\skyline dataminer\subdir\test.txt")]
[DataRow(@"\\127.0.0.1\c$\skyline dataminer", @"subdir\test.txt", @"\\127.0.0.1\c$\skyline dataminer\subdir\test.txt")]
public void ConstructSecurePathWithSubdirectoriesSuccess(string basePath, string filename, string expectedResult)
public void ConstructSecureFilePathWithSubdirectoriesSuccess(string basePath, string filename, string expectedResult)
{
string result = SecurePath.ConstructSecurePathWithSubDirectories(basePath, filename);
Assert.AreEqual(result, expectedResult);
Expand All @@ -49,7 +47,7 @@ public void ConstructSecurePathWithSubdirectoriesSuccess(string basePath, string
[DataRow(@"\\127.0.0.1\c$\skyline dataminer\test.txt", @"\\127.0.0.1\c$\skyline dataminer", @"test.txt")]
[DataRow(@"\\127.0.0.1\c$\skyline dataminer\subdir1\subdir2\test.txt", @"\\127.0.0.1\c$\skyline dataminer", @"subdir1", @"subdir2", @"test.txt")]
[DataRow(@"\\127.0.0.1\c$\skyline dataminer\subdir1\test.txt", @"\\127.0.0.1\c$\", @"skyline dataminer", @"subdir1", @"test.txt")]
public void ConstructSecurePathWithParamsSuccess(string expectedResult, params string[] paths)
public void ConstructSecureFilePathWithParamsSuccess(string expectedResult, params string[] paths)
{
string result = SecurePath.ConstructSecurePath(paths);
Assert.AreEqual(result, expectedResult);
Expand Down Expand Up @@ -109,7 +107,7 @@ public void ConstructSecurePathWithParamsSuccess(string expectedResult, params s
[DataRow(@"\\127.0.0.1\c$\skyline dataminer", @"%programfiles%\test.txt", typeof(InvalidOperationException))]
[DataRow(@"\\127.0.0.1\c$\skyline dataminer\", @"\\127.0.0.1\c$\skyline dataminer\test.txt", typeof(InvalidOperationException))]
[DataRow(@"\\127.0.0.1\c$\skyline dataminer", @"\\127.0.0.1\c$\skyline dataminer\subdir1\test.txt", typeof(InvalidOperationException))]
public void ConstructSecurePathFailure(string basePath, string filename, Type expectedExceptionType)
public void ConstructSecureFilePathFailure(string basePath, string filename, Type expectedExceptionType)
{
Action action = () => SecurePath.ConstructSecurePath(basePath, filename);
action.Should().Throw<Exception>().Which.Should().BeOfType(expectedExceptionType);
Expand Down Expand Up @@ -160,8 +158,8 @@ public void ConstructSecurePathFailure(string basePath, string filename, Type ex
[DataRow(@"\\127.0.0.1\c$\skyline dataminer", @"subdir1/test.txt", typeof(InvalidOperationException))]
[DataRow(@"\\127.0.0.1\c$\skyline dataminer", @"..\..\..\test.txt", typeof(InvalidOperationException))]
[DataRow(@"\\127.0.0.1\c$\skyline dataminer", @"%programfiles%\test.txt", typeof(InvalidOperationException))]
public void ConstructSecurePathWithSubDirectoriesFailure(string basePath, string relativePath, Type expectedExceptionType)
{
public void ConstructSecureFilePathWithSubDirectoriesFailure(string basePath, string relativePath, Type expectedExceptionType)
{
Action action = () => SecurePath.ConstructSecurePathWithSubDirectories(basePath, relativePath);
action.Should().Throw<Exception>().Which.Should().BeOfType(expectedExceptionType);
}
Expand Down Expand Up @@ -221,10 +219,91 @@ public void ConstructSecurePathWithSubDirectoriesFailure(string basePath, string
[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer\", @"subdir1", @"\\127.0.0.1\c$\", "test.txt")]
[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer\", @"subdir1", @"\\127.0.0.1\c$\test.txt")]
[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer\", @"%programdata%", @"test.txt")]
public void ConstructSecurePathWithParamsFailure(Type expectedExceptionType, params string[] paths)
public void ConstructSecureFilePathWithParamsFailure(Type expectedExceptionType, params string[] paths)
{
Action action = () => SecurePath.ConstructSecurePath(paths);
action.Should().Throw<Exception>().Which.Should().BeOfType(expectedExceptionType);
}

[TestMethod]
// straightforward cases
[DataRow(@"C:\skyline dataminer\folder", @"C:\skyline dataminer", "folder")]
[DataRow(@"C:\skyline dataminer\folder", @"C:\skyline dataminer", @"folder\")]
[DataRow(@"C:\skyline dataminer\folder", @"C:\skyline dataminer\", "folder")]
// straightforward cases with subdirectories
[DataRow(@"C:\skyline dataminer\subdir1\subdir2", @"C:\skyline dataminer", @"subdir1", @"subdir2\")]
[DataRow(@"C:\skyline dataminer\subdir1\subdir2", @"C:\skyline dataminer\", @"subdir1", @"subdir2")]
// edge case where one of the subdirs is absolute but points to the expected location
[DataRow(@"C:\skyline dataminer\subdir1", @"C:\", @"skyline dataminer", @"subdir1\")]
// network paths
[DataRow(@"\\127.0.0.1\c$\skyline dataminer\folder", @"\\127.0.0.1\c$\skyline dataminer", @"folder")]
[DataRow(@"\\127.0.0.1\c$\skyline dataminer\subdir1\subdir2", @"\\127.0.0.1\c$\skyline dataminer", @"subdir1", @"subdir2")]
[DataRow(@"\\127.0.0.1\c$\skyline dataminer\subdir1", @"\\127.0.0.1\c$\", @"skyline dataminer", @"subdir1\")]
public void ConstructSecureDirectoryPathWithParamsSucess(string expectedResult, params string[] paths)
{
string result = SecurePath.ConstructSecurePath(paths);
Assert.AreEqual(result, expectedResult);
}

[TestMethod]
////case with invalid arguments
//[DataRow(typeof(ArgumentException), null)]
//[DataRow(typeof(ArgumentException), @"C:\skyline dataminer\")]
//// case with invalid characters in basepath
//[DataRow(typeof(InvalidOperationException), @"C:/skyline dataminer/", @"subdir1", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer>\", @"subdir1", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer<\", @"subdir1", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer|\", @"subdir1", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), "C:\\skyline dataminer\0\\", @"subdir1", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"%programfiles%", @"subdir1", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer\..\", @"subdir1", @"subdir2")]
//// case with invalid characters in subdirectories
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer", @"subdir1/", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer\", @"subdir1>", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer\", @"subdir1<", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer\", @"subdir1|", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer\", "subdir1\0", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer\", "subdir1\"", @"subdir2")]
//// case with invalid characters in last path segment
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer", @"subdir1", @"/subdir2")]
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer\", @"subdir1", @"<subdir2")]
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer\", @"subdir1", @">subdir2")]
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer\", @"subdir1", @"|subdir2")]
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer\", @"subdir1", "\0subdir2")]
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer\", @"subdir1", "\"subdir2")]
//// case with directory traversal
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer\", @"subdir1", @"..", "subdir2")]
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer\", @"subdir1", @"C:\", "subdir2")]
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer\", @"subdir1", @"\\127.0.0.1\c$\", "subdir2")]
[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer\", @"subdir1", @"C:\subdir2")]
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer\", @"subdir1", @"\\127.0.0.1\c$\subdir2")]
//[DataRow(typeof(InvalidOperationException), @"C:\skyline dataminer\", @"%programdata%", @"subdir2")]
//// network path
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer>\", @"subdir1", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer<\", @"subdir1", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer|\", @"subdir1", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), "\\\\127.0.0.1\\c$\\skyline dataminer\0\\", @"subdir1", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer\..\", @"subdir1", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer", @"subdir1/", @"subdir2", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer\", @"subdir1>", @"subdir2", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer\", @"subdir1<", @"subdir2", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer\", @"subdir1|", @"subdir2", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer\", "subdir1\0", @"subdir2", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer\", "subdir1\"", @"subdir2", @"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer", @"subdir1", @"/subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer\", @"subdir1", @"<subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer\", @"subdir1", @">subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer\", @"subdir1", @"|subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer\", @"subdir1", "\0subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer\", @"subdir1", "\"subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer\", @"subdir1", @"..", "subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer\", @"subdir1", @"\\127.0.0.1\c$\", "subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer\", @"subdir1", @"\\127.0.0.1\c$\subdir2")]
//[DataRow(typeof(InvalidOperationException), @"\\127.0.0.1\c$\skyline dataminer\", @"%programdata%", @"subdir2")]
public void ConstructSecureDirectoryPathWithParamsFailure(Type expectedException, params string[] paths)
{
Action action = () => SecurePath.ConstructSecurePath(paths);
action.Should().Throw<Exception>().Which.Should().BeOfType(expectedException);
}
}
}
34 changes: 23 additions & 11 deletions SecureCoding/SecureIO/SecurePath.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public static SecurePath ConstructSecurePath(params string[] paths)
{
if (paths.Length < 2)
{
throw new ArgumentException("Paths array should at least contain the base path and the filename");
throw new ArgumentException("Paths array should contain at least 2 path segments");
}

var basePath = paths[0];
Expand All @@ -129,21 +129,20 @@ public static SecurePath ConstructSecurePath(params string[] paths)

for (int i = 1; i < paths.Length - 1; i++)
{
if (paths[i].ContainsInvalidPathCharacters())
{
throw new InvalidOperationException($"FileSystem.Instance.Path segment '{paths[i]}' contains invalid characters");
}
PathSegmentValidation(paths[i]);
}

if (FileSystem.Instance.Path.IsPathRooted(paths[i]))
var lastPathSegment = paths[paths.Length - 1];
if (lastPathSegment.IsFile())
{
if (lastPathSegment.ContainsInvalidFilenameCharacters())
{
throw new InvalidOperationException($"FileSystem.Instance.Path segment '{paths[i]}' cannot be a rooted path");
throw new InvalidOperationException($"Filename '{lastPathSegment}' contains invalid characters");
}
}

var filename = paths[paths.Length - 1];
if (filename.ContainsInvalidFilenameCharacters())
else
{
throw new InvalidOperationException($"Filename '{filename}' contains invalid characters");
PathSegmentValidation(lastPathSegment);
}

var combinedPath = FileSystem.Instance.Path.Combine(paths);
Expand Down Expand Up @@ -231,5 +230,18 @@ private static void DirectoryTraversalValidation(bool allowSubDirectories, strin
throw new InvalidOperationException("Sub-directories flag should be set to true in order to construct a path with sub-directories in filename");
}
}

private static void PathSegmentValidation(string pathSegment)
{
if (pathSegment.ContainsInvalidPathCharacters())
{
throw new InvalidOperationException($"FileSystem.Instance.Path segment '{pathSegment}' contains invalid characters");
}

if (FileSystem.Instance.Path.IsPathRooted(pathSegment))
{
throw new InvalidOperationException($"FileSystem.Instance.Path segment '{pathSegment}' cannot be a rooted path");
}
}
}
}
13 changes: 12 additions & 1 deletion SecureCoding/SecureIO/StringExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
namespace Skyline.DataMiner.Utils.SecureCoding.SecureIO
{
using System;
using System.IO;
using System.Linq;
using Skyline.DataMiner.CICD.FileSystem;

Expand Down Expand Up @@ -46,6 +45,18 @@ public static bool IsPathValid(this string path)
}
}

internal static bool IsFile(this string path)
{
try
{
return FileSystem.Instance.Path.HasExtension(path);
}
catch (Exception)
{
return false;
}
}

internal static bool ContainsInvalidPathCharacters(this string path)
{
return path.IndexOfAny(GetInvalidPathChars()) != -1
Expand Down