Skip to content

Commit

Permalink
(GH-341) Do not allow combining paths with colon
Browse files Browse the repository at this point in the history
When attempting to combine paths, do not allow any paths being added to
have colon `:` as that will reset the path. This can lead to possibly
very bad situations when an incorrect command is sent to choco.
  • Loading branch information
ferventcoder committed Jun 26, 2015
1 parent e4daf2d commit 270ea94
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace chocolatey.tests.infrastructure.filesystem
{
using System;
using System.IO;
using NUnit.Framework;
using Should;
using chocolatey.infrastructure.filesystem;
using chocolatey.infrastructure.platforms;
Expand Down Expand Up @@ -113,6 +114,13 @@ public void Combine_should_combine_when_paths_start_with_forwardslashes_in_subpa
"C:\\temp\\yo\\filename.txt"
: "C:/temp/yo/filename.txt");
}

[Fact]
[ExpectedException(typeof(ApplicationException), MatchType = MessageMatch.StartsWith, ExpectedMessage = "Cannot combine a path with")]
public void Combine_should_error_if_any_path_but_the_primary_contains_colon()
{
FileSystem.combine_paths("C:\\temp", "C:");
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,13 @@ public void handle_validation(ChocolateyConfiguration configuration)
{
throw new ApplicationException("Package name is required. Please pass at least one package name to install.");
}
// Need a better check on this before releasing. Issue will be covered by other fixes
//// investigate https://msdn.microsoft.com/en-us/library/system.io.path.getinvalidpathchars(v=vs.100).aspx
//if (configuration.PackageNames.Contains(":"))
//{
// throw new ApplicationException("Package name cannot contain invalid characters.");
//}

if (configuration.ForceDependencies && !configuration.Force)
{
throw new ApplicationException("Force dependencies can only be used with force also turned on.");
Expand Down
8 changes: 8 additions & 0 deletions src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ public string combine_paths(string leftItem, params string[] rightItems)
var combinedPath = Platform.get_platform() == PlatformType.Windows ? leftItem : leftItem.Replace('\\', '/');
foreach (var rightItem in rightItems)
{
if (rightItem.Contains(":")) throw new ApplicationException("Cannot combine a path with ':' attempted to combine '{0}' with '{1}'".format_with(rightItem, combinedPath));

var rightSide = Platform.get_platform() == PlatformType.Windows ? rightItem : rightItem.Replace('\\', '/');
if (rightSide.StartsWith(Path.DirectorySeparatorChar.to_string()) || rightSide.StartsWith(Path.AltDirectorySeparatorChar.to_string()))
{
Expand Down Expand Up @@ -336,6 +338,9 @@ public void create_directory(string directoryPath)

public void move_directory(string directoryPath, string newDirectoryPath)
{
if (string.IsNullOrWhiteSpace(directoryPath) || string.IsNullOrWhiteSpace(newDirectoryPath)) throw new ApplicationException("You must provide a directory to move from or to.");
if (combine_paths(directoryPath,"").is_equal_to(combine_paths(Environment.GetEnvironmentVariable("SystemDrive"),""))) throw new ApplicationException("Cannot move or delete the root of the system drive");

try
{
this.Log().Debug("Moving '{0}'{1} to '{2}'".format_with(directoryPath, Environment.NewLine, newDirectoryPath));
Expand Down Expand Up @@ -404,6 +409,9 @@ public void create_directory_if_not_exists(string directoryPath, bool ignoreErro

public void delete_directory(string directoryPath, bool recursive)
{
if (string.IsNullOrWhiteSpace(directoryPath)) throw new ApplicationException("You must provide a directory to delete.");
if (combine_paths(directoryPath, "").is_equal_to(combine_paths(Environment.GetEnvironmentVariable("SystemDrive"), ""))) throw new ApplicationException("Cannot move or delete the root of the system drive");

this.Log().Debug(() => "Attempting to delete directory \"{0}\".".format_with(get_full_path(directoryPath)));
allow_retries(() => Directory.Delete(directoryPath, recursive));
}
Expand Down

0 comments on commit 270ea94

Please sign in to comment.