HeliosVirtualCockpit / Helios

Helios Distribution
https://github.com/HeliosVirtualCockpit/Helios/wiki
GNU General Public License v3.0
194 stars 34 forks source link

writes to Documents\Helios denied #290

Open derammo opened 3 years ago

derammo commented 3 years ago

Discord user THOTH reports

image

derammo commented 3 years ago

root cause is failure to use proper localization for this path

localization is not the issue

image

we should spin a new 1630 I think if I can fix this today?

derammo commented 3 years ago

in Italian windows, "Documenti" is a junction to Documents

18/08/2019 13:48 Cookies [C:\Users\sal\AppData\Local\Microsoft\Windows\INetCookies] 18/08/2019 13:48 Dati applicazioni [C:\Users\sal\AppData\Roaming] 18/08/2019 13:48 Documenti [C:\Users\sal\Documents] 18/08/2019 13:48 Impostazioni locali [C:\Users\sal\AppData\Local] 18/08/2019 13:48 Menu Avvio [C:\Users\sal\AppData\Roaming\Microsoft\Windows\Start Menu] 18/08/2019 13:48 Modelli [C:\Users\sal\AppData\Roaming\Microsoft\Windows\Templates] 18/08/2019 13:48 Recenti [C:\Users\sal\AppData\Roaming\Microsoft\Windows\Recent] 18/08/2019 13:48 Risorse di rete [C:\Users\sal\AppData\Roaming\Microsoft\Windows\Network Shortcuts] 18/08/2019 13:48 Risorse di stampa [C:\Users\sal\AppData\Roaming\Microsoft\Windows\Printer Shortcuts] 18/08/2019 13:48 SendTo [C:\Users\sal\AppData\Roaming\Microsoft\Windows\SendTo]

after manually creating the viewport setups folder, this happens:

for unknown reasons, permission is denied to Helios for writing HeliosSettings.xml image

derammo commented 3 years ago

another report of a permission denied to HeliosSettings (Mongo52) from a user who was successfully (mostly) running 1.6 before and this went badly after they tried to reinstall a profile (A-10C)

internal notes:

1) develop diagnostic code to run on permission denied or failure to find/create folders

2) check if the removing the pre-create of folders from installer fixes this (could be a "install only for me" type thing?)

derammo commented 3 years ago

the second user's permissions problem was caused by BitDefender denying write access

the original Italian user has MalwareBytes

derammo commented 3 years ago

internal notes:

  1. test what sort of failure happens when BitDefender denies access. Can we tell the difference between permission denied from AV and actual permissions being wrong by inpecting effective file permissions? That would allow us to detect this A/V nonsense very early at start up and display a civilized message.
derammo commented 3 years ago

just learned that BitDefender, MalwareBytes, and even Windows itself ("Controlled Folder Access") try to prevent ransomware by disallowing writes to documents folders. And because they are garbage, they may restrict Helios from writing to its own folder it created during its install. What a freaking waste of time. My goal now is to detect something like that and throw up a box linking to a wiki page where we will collect information about how to configure whitelist exceptions for the various AV products.

Clarification: I will detect the inability to write and/or create folders on startup. We aren't going to detect AVs or probe for security settings because that will DEFINITELY tag us as an attacker.

derammo commented 3 years ago

it is possible this is just caused by us having created the Helios folder with the installer (which is pretty wrong as noted elsewhere in #280)

if the Helios folder is created by Helios on startup, then it looks like this

PS C:\Users\user\Documents> get-acl Helios |fl

Path   : Microsoft.PowerShell.Core\FileSystem::C:\Users\user\Documents\Helios
Owner  : DESKTOP-21FSDS6\user
Group  : DESKTOP-21FSDS6\None
Access : NT AUTHORITY\SYSTEM Allow  FullControl
         BUILTIN\Administrators Allow  FullControl
         DESKTOP-21FSDS6\user Allow  FullControl
Audit  :
Sddl   : O:S-1-5-21-272481886-374559052-1716243598-1001G:S-1-5-21-272481886-374559052-1716243598-513D:(A;OICIID;FA;;;SY
         )(A;OICIID;FA;;;BA)(A;OICIID;FA;;;S-1-5-21-272481886-374559052-1716243598-1001)

if it is created by the installer, then it looks like this:

PS C:\Users\user\Documents> get-acl Helios |fl

Path   : Microsoft.PowerShell.Core\FileSystem::C:\Users\user\Documents\Helios
Owner  : NT AUTHORITY\SYSTEM
Group  : NT AUTHORITY\SYSTEM
Access : NT AUTHORITY\SYSTEM Allow  FullControl
         BUILTIN\Administrators Allow  FullControl
         DESKTOP-21FSDS6\user Allow  FullControl
Audit  :
Sddl   : O:SYG:SYD:(A;OICIID;FA;;;SY)(A;OICIID;FA;;;BA)(A;OICIID;FA;;;S-1-5-21-272481886-374559052-1716243598-1001)

Note that it is owned by the system when it is created by the installer. That's a pretty good reason for an antivirus to feel we should not be writing to it.

derammo commented 3 years ago

it is not obvious why this problem would only happen with 1.6 and not with 1.4

even reinstalling 1.4 appears to fix it. certainly 1.4 has to write HeliosSettings.xml and log files also, so why is 1.6 different?

verified in a VM that 1.4 doesn't create this folder any differently either

derammo commented 3 years ago

maybe it is just a function of 1.6 looking a bit more evil to the anti viruses because of our elevation code and/or other code? see #292

derammo commented 3 years ago

just purchased AVG full version and installed it on a VM. immediately get this: image

derammo commented 3 years ago

ok I have a very strong explanation for why 1.4 works and 1.6 does not by default and it's simple. Because we moved the exe files to a new location (the program files is moved) any existing exceptions / white list entries do not apply to Helios 1.6. I just installed 1.6, uninstalled, installed1.4, and then reinstalled 1.6. It remembered the 1.6 white list entries but required new ones for the 1.4 executables. So basically it is just based on exe path.

asking user to white list profile editor and control center 1.6

derammo commented 3 years ago

also still testing MalwareBytes and BitDefender

derammo commented 3 years ago

testing with various AV vendors doesn't show anything. Maybe it matters if the files were previously installed by 1.4 or not. Maybe there are other heuristics in play.

At this point I will just improve the checking for access on start and reporting of any problems related to permission denied.

derammo commented 3 years ago

cant even repro the problem reported by a bitdefender user with the commercial version of bitdefender. I am not planning to do any more work other than better reporting

derammo commented 3 years ago

found a configuration that repros this: set up windows with "Controlled Folder Access"

profile editor crashes on attempt to create Helios folder. We should develop a probe for writable, then have Helios open a wiki page and exit gracefully or maybe even wait and try again after user says they have given permission.

derammo commented 3 years ago

weirdly it let the elevated HeliosPatching run without blocking

derammo commented 3 years ago

control center starts even after having been denied (does not create its log)

derammo commented 3 years ago

control center causes a firewall popup on start profile

derammo commented 3 years ago

opening a web page might look even worse from a security perspective. For now, this will just display an informative message

image

derammo commented 3 years ago

also note that we cannot log this

derammo commented 3 years ago

added another commit to also mention HeliosPatching.exe in the message e0ae232b93df65ba9ddd2aec2725a88a88cbc45d

derammo commented 3 years ago

if this gets worse, do we have to consider something like AppData\Roaming or AppData\Local? What are the default backup inclusions etc? Putting our stuff in a folder that is by default hidden on a normal Users's machine seems terrible.

derammo commented 3 years ago

we could have an option for this. If Helios can't write during start up, it could offer to start using an alternate location. We would have to use a registry key to remember this setting. Presumably, we would be allowed to copy our stuff to the new location as long as we don't try to open any files in a writable mode.

derammo commented 3 years ago

rescheduling because this needs more study. mitigated in Release1632 by testing for write and displaying a better message

derammo commented 3 years ago

actually, the obvious choice is probably "Saved Games\Helios". It will result in gigabytes (if you have multiple CZ profiles) of files added to Saved Games, which is probably ok. Some large DCS mods go to there also.

Backup inclusion should be ok. This would be easy enough to test, with a command line argument to run with document path homed in Saved Games now. The current arguments only allow the selection of directories in Documents; this would be a new switch.

derammo commented 3 years ago

default folders protected by Windows feature:

Documents Pictures Videos Music Favorites

derammo commented 3 years ago

there have not been any additional reports of problems, so maybe we just stick with what we have for now