(GH-387) Validate before remove rollback folder

As a further enhancement for GH-341 (270ea94), ensure that package names are not attempting to navigate out of the lib backup directory. A specially crafted package name could cause choco to attempt to delete folders it should not, therefore we need to restrict it to the lib backup folder only. If we find we are no longer in a subdirectory of the backup directory, we should return immediately without attempting to delete anything.
chocolatey · Sep 18, 2015 · bb270cb · bb270cb
1 parent 344268b
commit bb270cb
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 3 deletions.
diff --git a/src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs b/src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs
@@ -703,7 +703,10 @@ private void rollback_previous_version(ChocolateyConfiguration config, PackageRe
                 }
             }
 
+            rollbackDirectory = _fileSystem.get_full_path(rollbackDirectory);
+
             if (string.IsNullOrWhiteSpace(rollbackDirectory) || !_fileSystem.directory_exists(rollbackDirectory)) return;
+            if (!rollbackDirectory.StartsWith(ApplicationParameters.PackageBackupLocation) || rollbackDirectory.is_equal_to(ApplicationParameters.PackageBackupLocation)) return;
 
             this.Log().Debug("Attempting rollback");
 

diff --git a/src/chocolatey/infrastructure.app/services/NugetService.cs b/src/chocolatey/infrastructure.app/services/NugetService.cs
@@ -376,7 +376,7 @@ public ConcurrentDictionary<string, PackageResult> install_run(ChocolateyConfigu
 
         public void remove_rollback_directory_if_exists(string packageName)
         {
-            var rollbackDirectory = _fileSystem.combine_paths(ApplicationParameters.PackageBackupLocation, packageName);
+            var rollbackDirectory = _fileSystem.get_full_path(_fileSystem.combine_paths(ApplicationParameters.PackageBackupLocation, packageName));
             if (!_fileSystem.directory_exists(rollbackDirectory))
             {
                 //search for folder
@@ -385,10 +385,13 @@ public void remove_rollback_directory_if_exists(string packageName)
                 {
                     rollbackDirectory = possibleRollbacks.OrderByDescending(p => p).DefaultIfEmpty(string.Empty).FirstOrDefault();
                 }
-            }
 
+                rollbackDirectory = _fileSystem.get_full_path(rollbackDirectory);
+            }
+
             if (string.IsNullOrWhiteSpace(rollbackDirectory) || !_fileSystem.directory_exists(rollbackDirectory)) return;
-
+            if (!rollbackDirectory.StartsWith(ApplicationParameters.PackageBackupLocation) || rollbackDirectory.is_equal_to(ApplicationParameters.PackageBackupLocation)) return;
+
             FaultTolerance.try_catch_with_logging_exception(
                 () => _fileSystem.delete_directory_if_exists(rollbackDirectory, recursive: true),
                 "Attempted to remove '{0}' but had an error:".format_with(rollbackDirectory),