search

NuGet / Home

Repo for NuGet Client issues
Other
1.5k stars 253 forks source link

Security: Should random code execution in target files be at least something you warn about? #10262

Open drauch opened 3 years ago

drauch commented 3 years ago

Remove the content above here and fill out details below.

Details about Problem

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

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

dotnet.exe --version (if appropriate): doesn't matter

VS version (if appropriate): doesn't matter

OS version (i.e. win10 v1607 (14393.321)): doesn't matter

Worked before? If so, with which NuGet version: has probably always been a problem

Detailed repro steps so we can see the same problem

  1. Install https://www.nuget.org/packages/IAmRoot/ NuGet package

  2. Build your solution

  3. Some C# code hidden away in a targets file in the NuGet package is executed on my machine without me noticing

...

Sample Project

See https://github.com/augustoproiete/i-am-root-nuget-package

To me this sounds like a security flaw in the NuGet design. My understanding was that this is not possible anymore after you removed support for Install/Uninstall.ps1 in NuGet packages. Is there some documentation about the consequences? Is it possible to add warnings if there are target files in the package maybe even only if target files contain arbitrary C# code and/or use tasks which try to go outside the solution directory?

I haven't thought about all the consequences yet, but this sounds like a problem to me.

Best regards, D. R.

zkat commented 3 years ago

@JonDouglas @kartheekp-ms can you look into this discussion? There's a lot to consider with this issue and we want to make sure we have clarity about where we stand on this, and whether we should do anything about this particular corner case.

erdembayar commented 3 years ago

@JonDouglas @kartheekp-ms @aortiz-msft I tested this one, it does what it says, when you rebuild/clean project it opens new tab in your default browser with "Root image" from movie, technically this one can do whatever it wants here. It's valid thing, also it's part of far greater problem. Because any code you download from internet can include hidden injections does something you doesn't want. It applies not only to build project process demod here, also code injection happen later during runtime. Besides above proposal one possible solution would be checking for this problem when user upload the package first time to nuget.org, but it doesn't work for private nuget package hostings.

kartheekp-ms commented 3 years ago

I can reproduce this issue. MSBuild inline tasks are valid in .targets file. Whether NuGet should support this feature in .targets file in a package is an open question for now.

tagging @rainersigwald to get his perspective on this bug from msbuild side. tagging @drewgillies as he is working on an epic to flag vulnerable packages.

JonDouglas commented 3 years ago

I think there's a longer term question here. By removing the ability for code execution in targets to happen, I believe we will break many packages, tools, etc that heavily rely on this functionality without an alternative. We may need see what exactly that might mean impact wise & also functionality wise. We should also reach out to our resident security experts to understand what is at risk here given many ecosystems allow arbitrary code execution.

KalleOlaviNiemitalo commented 3 years ago

Does devcontainer help against this risk?