diff --git a/SecureCoding.Test/SecureCoding.Test.csproj b/SecureCoding.Test/SecureCoding.Test.csproj index 87ad2c2..0fe0dc2 100644 --- a/SecureCoding.Test/SecureCoding.Test.csproj +++ b/SecureCoding.Test/SecureCoding.Test.csproj @@ -15,8 +15,8 @@ - - + + diff --git a/SecureCoding.Test/SecureIO/SecurePathTests.cs b/SecureCoding.Test/SecureIO/SecurePathTests.cs index 748e2b8..7445640 100644 --- a/SecureCoding.Test/SecureIO/SecurePathTests.cs +++ b/SecureCoding.Test/SecureIO/SecurePathTests.cs @@ -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] @@ -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); @@ -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); @@ -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); @@ -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().Which.Should().BeOfType(expectedExceptionType); @@ -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().Which.Should().BeOfType(expectedExceptionType); } @@ -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().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", "\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", "\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().Which.Should().BeOfType(expectedException); + } } } \ No newline at end of file diff --git a/SecureCoding/SecureIO/SecurePath.cs b/SecureCoding/SecureIO/SecurePath.cs index 70e50bc..162ce8d 100644 --- a/SecureCoding/SecureIO/SecurePath.cs +++ b/SecureCoding/SecureIO/SecurePath.cs @@ -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]; @@ -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); @@ -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"); + } + } } } \ No newline at end of file diff --git a/SecureCoding/SecureIO/StringExtensions.cs b/SecureCoding/SecureIO/StringExtensions.cs index 622ad07..8e2bdcd 100644 --- a/SecureCoding/SecureIO/StringExtensions.cs +++ b/SecureCoding/SecureIO/StringExtensions.cs @@ -1,7 +1,6 @@ namespace Skyline.DataMiner.Utils.SecureCoding.SecureIO { using System; - using System.IO; using System.Linq; using Skyline.DataMiner.CICD.FileSystem; @@ -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