Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

choco install -y C: deletes all files #341

Closed
bill-long opened this issue Jun 25, 2015 · 5 comments
Closed

choco install -y C: deletes all files #341

bill-long opened this issue Jun 25, 2015 · 5 comments

Comments

@bill-long
Copy link

If you run choco install -y C:, all files are deleted from the folder where you ran this command.

In remove_rollback_directory_if_exists(), the packageName being passed in is "C:":

        public void remove_rollback_directory_if_exists(string packageName)
        {
            var rollbackDirectory = _fileSystem.combine_paths(ApplicationParameters.PackageBackupLocation, packageName);

So rollbackDirectory is now set to "C:". Finally we end up in chocolatey.infrastructure.filesystem.DotNetFileSystem.delete_directory(), with a directoryPath of "C:" and recursive set to true. Then we call:

allow_retries(() => Directory.Delete(directoryPath, recursive));

I'm a little surprised this doesn't actually wipe out everything starting from the root of C: in my tests, but it seems that it hits an exception trying to delete the folder where the command is being run. So maybe the exception saves you if you happen to run this from a subfolder with nothing important in it.

Of course, if you actually run this command from the root of C:, then it makes quite a mess of things.

@ferventcoder
Copy link
Member

Thanks for investigating the reason @bill-long - this will be something to fix quite quickly!

ferventcoder added a commit that referenced this issue Jun 27, 2015
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.
ferventcoder added a commit that referenced this issue Jun 27, 2015
* stable:
  (GH-341) Do not allow combining paths with colon
  (spec) don't allow interactive windows in specs
  (GH-219)(GH-56) Allow PowerShell interaction
  (spec) Set uninstall base scenario
  (GH-305) add warning if application not uninstalled

Conflicts:

src/chocolatey.tests.integration/infrastructure/commands/CommandExecutorSpecs.cs
	src/chocolatey.tests/infrastructure/filesystem/DotNetFileSystemSpecs.cs
	src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs
	src/chocolatey/infrastructure/commands/CommandExecutor.cs
@ferventcoder
Copy link
Member

image

@ferventcoder
Copy link
Member

For posterity, this was discovered based on the LabTech plugin for choco noted https://gitter.im/chocolatey/choco?at=558b84c638e37bf74261d3bd

@ferventcoder ferventcoder self-assigned this Jun 27, 2015
@ADTC
Copy link

ADTC commented Aug 5, 2015

That C: points to the current working directory in C drive is a well-known anomaly in MS-DOS and Windows. If you want to refer to the root of C drive, you have to specify C:\

Having that said, I wonder what would have happened if you had done choco install -y C:\ (in the unfixed version).

@ferventcoder
Copy link
Member

Note that there may be a fix if you are affected by this: http://www.squidworks.net/2015/06/labtech-chocolatey-nuget-ii-plugin/#comment-17200

From Ben Perkins:

If anyone else runs into this issue, we found a much better fix than reloading everything. Boot the effected machines to a windows 2 go type of portable windows install. Log in as admin. Use Shadow copy and pick a restore date prior too the issue. The only effected directory really seemed to be \Windows as all user data and program data appeared to be intact. After you extract the Shadow copy, copy the Windows directory minus the temp and sys32 folders and skip any of the same files. Then copy the sys32 folder over top of the sys32 folder of the effected machine. This was successful 90% of the time and all that was left were a few domain trust issues.

ferventcoder added a commit that referenced this issue Sep 18, 2015
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants