mandiant / flare-vm

A collection of software installations scripts for Windows systems that allows you to easily setup and maintain a reverse engineering environment on a VM.
Apache License 2.0
6.22k stars 887 forks source link

Installation fails with Desktop\config.xml as custom config file #537

Closed Ana06 closed 7 months ago

Ana06 commented 8 months ago

What's the problem?

If you create a custom config file in the Desktop with the name config.xml and execute the installer script, the Get-ConfigFile function (introduced in https://github.com/mandiant/flare-vm/pull/531) will fail trying to copy the file to itself with the following error:

Copy-Item : Cannot overwrite the item C:\Users\flare\Desktop\config.xml with itself.
At C:\Users\flare\Desktop\install.ps1:97 char:9
+         Copy-Item -Path $fileSource -Destination $fileDestination -Fo ...
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : WriteError: (C:\Users\flare\Desktop\config.xml:String) [Copy-Item], IOException
    + FullyQualifiedErrorId : CopyError,Microsoft.PowerShell.Commands.CopyItemCommand

Steps to Reproduce

  1. Download config.xml and install.ps1 to the Desktop (the name and location of config.xml are important to reproduce the error.
  2. Run the installer script with .\install.ps1 -customConfig config.xml -noGui

Environment

It is not environment specific.

Fix proposals

Workaround

Moving the config file to a different location (different from the Desktop) or renaming the config file allows to run the install script sucessfully.

Fix proposal 1

Add a check to the Get-ConfigFile function to avoid copying the file if source and destination are the same. This has the problem that we duplicate the config file in the Desktop when providing an argument.

Fix proposal 2

Do not copy the file in Get-ConfigFile as we did before https://github.com/mandiant/flare-vm/pull/531

emtuls commented 8 months ago

Would changing Copy-Item to Move-Item be a viable option here?

It would mitigate the duplication caused by copying and should also mitigate the error.

Ana06 commented 7 months ago

Would changing Copy-Item to Move-Item be a viable option here?

yes, it is similar to my option 1 to not copy the file in this case. Not sure if it would be confusing for the user, but having two files may also be confusing.

Ana06 commented 7 months ago

I wonder also if we should move the config file to the VM folder where we store the background picture and other files already instead of using the Desktop.