msys2/msys2-installer

CVE-2022-37172

lazka opened this issue · 4 comments

lazka commented

https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-37172

So there is an issue to track this.

I couldn't find any other installer that handles this, so I'm not sure what the right thing to do is (except cygwin, which seems to set some hardcoded ACLs, but I couldn't find their code/logic)

The only thing I can think of is:

icacls.exe C:/msys64 //reset # just for debugging, this goes to the default we have now
icacls.exe C:/msys64 //grant "$USERDOMAIN\\$USERNAME:F" # full permissions for the installing user
icacls.exe C:/msys64 //inheritancelevel:r # disable inheritance, remove inherited permissions

Though not sure if we have the current user somehow in the installer, and if that breaks anything else. It might give more permissions as before when installing into a more strict directory (??)

On could argue that this is how Windows works.. you will get the permissions of the directory you install to, and if C is world writable, then MSYS2 is too. But I understand the the defaults of both Windows and our installer make this insecure.

Ideas / thoughts / and pointers to other projects welcome.

elieux commented

It's an interesting combination of circumstances:

  • the drive root is world-writable most probably due to compatibility concerns
  • due to how UAC works and maybe some other factors, requiring admin permissions by itself won't help
  • the Program Files directory, which has correct permissions, has a space in it for which is a mortal enemy of much of the software we distribute

The defaults:

> icacls C:\
C:\ BUILTIN\Administrators:(OI)(CI)(F)
    NT AUTHORITY\SYSTEM:(OI)(CI)(F)
    BUILTIN\Users:(OI)(CI)(RX)
    NT AUTHORITY\Authenticated Users:(OI)(CI)(IO)(M)
    NT AUTHORITY\Authenticated Users:(AD)
    Mandatory Label\High Mandatory Level:(OI)(NP)(IO)(NW)

First three (admins, system and users) are very reasonable, the two after (authenticated users) are the contentious ones, the last one (label) shouldn't play a role. I'm not quite knowledgeable enough to say whether read access for authenticated users is required for anything; in my experience, everything works without it.

Note that the creator/owner will have implicit full access.

Proposal

It depends on how complex we want this. The proposal above is essentially an installation only for that singular user and no one and nothing else. I'd add at least the system account.

We should definitely give a choice. I guess we could have three modes:

  1. keep inherited, as before
  2. per-user install (should not be admin): grant full access to system, maybe full access to self, remove inherited
  3. system-wide install (requires admin): grant full access to system, admins, read to users, remove inherited

We could preselect the mode according to the installation path and current user, but that's already a bit further than this proposal should probably go. :)

I'm not sure how to design the whole process around it though. I'd like to present the permissions (to see the consequences of choosing mode 1) to the user somehow (maybe just give a button that opens the Explorer properties of the directory with the Security tab active), but we'd need to create the directory first, which means get the path first and possibly elevate as well.

lazka commented

I'm afraid that giving users a choice will (a) just confuse them and (b) make them select the default anyway.

What about just doing (2) always and have a page in the documentation for alternative recommended setups?

I assume most users are on a single user machine, so they shouldn't see any difference compared to now with (2) right?

goyalyashpal commented

Link to new page: https://www.cve.org/CVERecord?id=CVE-2022-37172

Umh, while I agree that most users are on a single user machine... I thought to share my perspective and reasoning behind:

  • I have 2 users: one admin (which I rarely use), and other: normal user. This is generally recommended to improve the security and managing the access level of the system.
  • I am gradually moving towards doing installs in non C:/ drive, and I am just about to install msys2 in non-C drive as well
  • Multi-user settings are fairly common in institutional settings, but this can be ignored as CLI is not the focus in those settings.