oneclick / rubyinstaller2

MSYS2 based RubyInstaller for Windows
https://rubyinstaller.org
BSD 3-Clause "New" or "Revised" License
648 stars 247 forks source link

Questions about automatic "system gemrc" file creation #388

Open deivid-rodriguez opened 1 month ago

deivid-rodriguez commented 1 month ago

What problems are you experiencing?

No problems, but I was investigating an issue about the etc gem being activated too soon on Windows, and eventually run into some code in the operating_system.rb file shipped by RubyInstaller2 that automatically creates an empty "system gemrc" file if it does not exist already:

https://github.com/oneclick/rubyinstaller2/blob/3f896e7e05d196b3a9c4a57fa46ecb38938660b8/resources/files/operating_system.rb#L56-L74

I got curious about the potential security issue. Does it affect more platforms other than Windows? The way I see it, system configuration is precisely intended for sharing a configuration with all users, so it's intended that a user with permissions to write files in Gem::ConfigFile::SYSTEM_WIDE_CONFIG_FILE can setup RubyGems configurations for all users. Also, it seems that the current implementation will still respect this global file if it's already there.

So I'm not fully clear about the security issue and I'm looking for a bit more insights.

Steps to reproduce

Create an empty Gemfile and observe that ruby -rbundler/setup -e 'puts Gem.loaded_specs["etc"]' activates the etc gem on Windows.

larskanis commented 1 month ago

Gem::ConfigFile::SYSTEM_WIDE_CONFIG_FILE refers to C:/ProgramData on Windows. Everyone can create a new file in this directory, but the file can not be changed by anyone other than the owner subsequently. But everyone can read the file.

This is a security risk, since Rubygems reads this gemrc file, if it is present. When an attacker creates this file, it can hijack the configuration of some other user. It could inject different sources urls for instance.

I had a discussion with @hsbt and @unak about this topic on hackerone in 2022. They concluded about not to change anything in Rubygems. So I went further and implemented this simple solution, that the very first call of gem creates this gemrc file and closes the attack vector.

The issue doesn't affect platforms other than Windows.

larskanis commented 1 month ago

We had an issue with fiddle in the past, that looks like this issue with etc loaded too soon: https://github.com/oneclick/rubyinstaller2/issues/251 . The way I solved it was to build a RubyInstaller specific Ruby C extension, which can call the Win32 API functions without fiddle.

Now that etc is gemified, we have the same issue here. The etc gem loading is triggered by accessing the constant Gem::ConfigFile::SYSTEM_WIDE_CONFIG_FILE. The etc gem reads the path here. But it is usually alternatively available in the environment variable ProgramData. So I think about switching to ENV['ProgramData'] in operating_system.rb.

deivid-rodriguez commented 1 month ago

Gem::ConfigFile::SYSTEM_WIDE_CONFIG_FILE refers to C:/ProgramData on Windows. Everyone can create a new file in this directory, but the file can not be changed by anyone other than the owner subsequently. But everyone can read the file.

Thanks for the explanation, yeah, that's what I imagined. Are there paths on Windows that can be changed only by administrators ("root users")? I'd expect the system wide configuration to be writable only by administrators, and I'd say that's the main issue here. I wonder, for example, which path does git use for its system configuration.

Should we use an alternative secure path for this "system wide config"? Should we refuse to load it at all if it's parent directory is writable by the current user?

In any case, I still don't fully understand the solution. If the file is hijacked before RubyGems is loaded, then the solution won't be effective, right? Shouldn't the file be unconditionally overwritten for the hack to be effective? How do we distinguish between a well intended system configuration vs a malicious attempt to hijack configuration?

Now that etc is gemified, we have the same issue here. The etc gem loading is triggered by accessing the constant Gem::ConfigFile::SYSTEM_WIDE_CONFIG_FILE. The etc gem reads the path here. But it is usually alternatively available in the environment variable ProgramData. So I think about switching to ENV['ProgramData'] in operating_system.rb.

That makes sense although it'd mean having duplication with the etc gem, so maybe there'd be risk that the hack becomes no longer effective if the etc gem eventually changes?