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

Update SharpCompress to 0.23.0 #1511

Closed
wants to merge 1 commit into from

Conversation

Thieum
Copy link
Contributor

@Thieum Thieum commented Jul 5, 2019

based on #1362
see #897

@Keboo
Copy link
Contributor

Keboo commented Aug 14, 2019

@shiftkey is there anything I can do that would help move this PR forward?

@pcrosthwaite
Copy link

amy idea's on when this will be merged in? it would remove the current high severity vulnerability with sharpcompress

@anaisbetts
Copy link
Contributor

anaisbetts commented Nov 4, 2019

Once again, this PR will break Squirrel, we've tried it before

#1362 (comment)

Here's a list of all the people who shipped bad updates then got to tell their users to manually update:

#1253

@thecodetinker
Copy link

Hello,

I have been following this for about a year since the SharpCompress vulnerability was first flagged by our automated checker. I understand that Squirrel is now stuck between a rock and a hard place, but it would be good to gather some ideas on how the impasse might be resolved. One cohort of users is happy to live with the security vulnerability because the headache of manually updating is the greater problem. But for another group, the reverse is true.

I wonder if one way through would be to create Squirrel 2.0, which would allow a path for upgrading SharpCompress without breaking anyone on 1.8.x? Or maybe a parallel build that contains the newer library? I do understand that Squirrel got burned badly last time, but sticking with SharpCompress 17.1 for ever doesn't seem viable.

As always, much thanks to all contributors. I know this is a tricky one, and am grateful for all the hard work on the project!

@anaisbetts
Copy link
Contributor

anaisbetts commented Nov 18, 2019

The fundamental problem is that NuGet itself breaks on zip files generated by new SharpCompress - this isn't a "We don't want to ship breaking changes" issue. This is a "Squirrel doesn't work anymore" issue.

At the end of the day, this security issue just does not apply to Squirrel - if you can control the zip file, you don't need a Zip exploit to gain privileges, you just....add an executable. This is not a security boundary we care about.

The viable options here are:

  1. Move to some completely different Zip library that generates zip files that are acceptable to NuGet
  2. Figure out why SharpCompress is breaking NuGet's zip parsing (difficult because it calls into a closed-source binary), and fix it / change options to make it work
  3. Leave Squirrel as-is, and tell your security policy team to get a Xanax prescription :)

@thecodetinker
Copy link

Oh I see, finally! That makes much more sense now. Are there items open for options 1 and 2? Meanwhile I'm off to the nearest chemist.

Thanks again.

@eugbaranov
Copy link

For what it's worth, nuget.exe internally uses System.IO.Compression.ZipArchive for building packages.
If I had to guess, the introduction of SharpCompress predates those APIs.

@anaisbetts, would you consider ZipArchive as a potential replacement of SharpCompress?

@alexThoerner
Copy link

Hello,

sorry to re-open this discussion and as a user of squirrel thank you for all the great work. Implementing auto-updates with squirrel is really easy and great fun and I really enjoy using it.

However, I do not really feel comfortable using a vulnerable library in a product, although I understood the explanation why this does not apply to squirrel. So are there any plans to use another ZIP library as replacement for SharpCompress ?

Thank you!

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

Successfully merging this pull request may close these issues.

7 participants