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

Permissions on files created inside ~/.nuget are too open #7673

Closed
gorazdzagar opened this issue Jan 7, 2019 · 9 comments
Closed

Permissions on files created inside ~/.nuget are too open #7673

gorazdzagar opened this issue Jan 7, 2019 · 9 comments
Assignees
Labels
Milestone

Comments

@gorazdzagar
Copy link

Details about Problem

NuGet product used (NuGet.exe | VS UI | Package Manager Console | dotnet.exe):

NuGet version (x.x.x.xxx):

dotnet.exe --version (if appropriate): 2.1.301

VS version (if appropriate):

OS version (i.e. win10 v1607 (14393.321)): Ubuntu 16.04

Worked before? If so, with which NuGet version:

Detailed repro steps so we can see the same problem

  1. dotnet restore

Files generated in ~/.nuget folder have world-wide writable permissions. Seems like umask set on a user executing the command is not respected. Any file created inside ~/.nuget should have permissions more restrictive.

@zivkan
Copy link
Member

zivkan commented Jan 9, 2019

@gorazdzagar I can't reproduce this. Can you provide any more information?

I created a brand new Ubuntu 16.04 LTS VM, then installed the dotnet CLI with the following instructions: https://dotnet.microsoft.com/download/linux-package-manager/ubuntu16-04/sdk-2.2.102. I ran dotnet new mvc to create a new project, which restores packages, then I got the following results:

zivkan@zivkan002:~/mvcapp$ ls -lad ~/.nuget/
drwxrwxr-x 3 zivkan zivkan 4096 Jan  9 23:47 /home/zivkan/.nuget/
zivkan@zivkan002:~/mvcapp$ rm -rf ~/.nuget/
zivkan@zivkan002:~/mvcapp$ umask 0077
zivkan@zivkan002:~/mvcapp$ dotnet restore
  Restore completed in 47.86 ms for /home/zivkan/mvcapp/mvcapp.csproj.
zivkan@zivkan002:~/mvcapp$ ls -lad ~/.nuget/
drwx------ 3 zivkan zivkan 4096 Jan  9 23:48 /home/zivkan/.nuget/

As you can see, NuGet restore is respecting umask on my machine.

I also did a search though NuGet's source code, but I couldn't find any code that changed file flags/permissions or ACLs.

@gorazdzagar
Copy link
Author

Hi @zivkan,

thank you for your reply. I'll provide a use case for it. We have umask 027 set in /etc/profile which applies to all users when shell is executed. What we are observing is files generated inside ~/.nuget/packages/ have world-write permission set where as they shouldn't. Could you please further test in your Ubuntu VM for package files not just the main folder.

Thank you

@zivkan zivkan added this to the Backlog milestone Jan 10, 2019
@zivkan zivkan added the Priority:2 Issues for the current backlog. label Jan 10, 2019
@zivkan
Copy link
Member

zivkan commented Jan 10, 2019

This issue should be fixed at the same time as #6778 and is related to #4424. The reason I couldn't find any code related to permissions earlier is because the fix doesn't use .NET APIs, it uses P/Invoke to run chmod directly on libc.

I don't sufficiently understand #4424, so I fear making any changes in case I regress the earlier bug. The team should discuss this.

@gorazdzagar
Copy link
Author

gorazdzagar commented Jan 11, 2019

I thank you for acknowledging this as a bug. If it helps further, here are the steps to reproduce:

umask 0027
dotnet restore

Check inside ~/.nuget/, example package files for ~/.nuget/packages/system.threading.tasks.extensions/4.5.0
drwxr-x--- 4 root root   4096 Jan 11 11:06 .
drwxr-x--- 3 root root   4096 Jan 11 11:06 ..
drwxr-x--- 7 root root   4096 Jan 11 11:06 lib
-rwxrw-rw- 1 root root   1139 May 15  2018 LICENSE.TXT
drwxr-x--- 6 root root   4096 Jan 11 11:06 ref
-rwxrw-rw- 1 root root  22354 Dec 17 15:55 .signature.p7s
-rw-r----- 1 root root 122000 Jan 11 11:06 system.threading.tasks.extensions.4.5.0.nupkg
-rw-r----- 1 root root     88 Jan 11 11:06 system.threading.tasks.extensions.4.5.0.nupkg.sha512
-rwxrw-rw- 1 root root   2633 May 15  2018 system.threading.tasks.extensions.nuspec
-rwxrw-rw- 1 root root  15835 May 15  2018 THIRD-PARTY-NOTICES.TXT
-rwxrw-rw- 1 root root      0 May 15  2018 useSharedDesignerContext.txt
-rwxrw-rw- 1 root root     42 May 15  2018 version.txt

1.) files should not have +x permission
2.) files absolutely must not have o+rw permission
3.) umask is respected on some files generated but not all

This is a severe security risk as anyone can change the content of an executable file. If root executes a file without prior knowledge of its content, this could be exploitable.

@zivkan zivkan modified the milestones: Backlog, 5.0 Mar 12, 2019
@zivkan zivkan added Type:Bug and removed Triage:NeedsTriageDiscussion Type:DCR Design Change Request labels Mar 12, 2019
@zivkan zivkan self-assigned this Mar 12, 2019
@zivkan
Copy link
Member

zivkan commented Mar 12, 2019

Today we released a fix for all affects products (nuget.exe, dotnet cli/sdk, VS for Mac, Mono): https://portal.msrc.microsoft.com/en-us/security-guidance/advisory/CVE-2019-0757

The fixed binaries do not change permissions of existing files on disk, so you should either delete the files and restore again, or fix the permissions yourself. But the updated binaries will now respect umask when creating new files.

@gorazdzagar thank you for helping us make a better product!

@zivkan zivkan closed this as completed Mar 12, 2019
@rrelyea
Copy link
Contributor

rrelyea commented Mar 22, 2019

@gorazdzagar - thanks very much for bringing this to our attention.
@zivkan - thanks for hearing him...and getting this released for 4.3.x, 4.4.x, 4.5.x, 4.6.x, 4.7.x, 4.8.x, 4.9.x, 5.0 -- including coordination with mono, vs4mac, and dotnet sdk!

@apoleon
Copy link

apoleon commented Mar 31, 2019

Hello, would it be possible to reference the fixing commit? Are older versions of nuget affected too (< 4.x)?

@zivkan
Copy link
Member

zivkan commented Apr 1, 2019

Here's the commit. NuGet only started modifying extracted file permissions because .NET Core 1.x would create (all) files with the u+x bit set, but .NET Core 2.x stopped, and some people were shipping tools (such as build tools, but now .NET global tools are also relevant) that needed the execute bit set, so NuGet started setting the execute bit from NuGet 4.3. For this reason, NuGet older than 4.3 is not affected, but also does not set the execute bit on extracted files.

@apoleon
Copy link

apoleon commented Apr 1, 2019

Thank you for the clarification. Debian is still shipping an old version of NuGet and I couldn't reproduce the problem. That makes sense now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants