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

Security issues with p7zip #45397

Closed
mbauman opened this issue May 20, 2022 · 13 comments
Closed

Security issues with p7zip #45397

mbauman opened this issue May 20, 2022 · 13 comments
Labels
security System security concerns and vulnerabilities

Comments

@mbauman
Copy link
Sponsor Member

mbauman commented May 20, 2022

p7zip has some active CVEs. I see we’re already carrying some of the patches for p7zip:

But I think we need patches for

The upstream p7zip project seems to be fairly abandoned, with lots of conflicting information about the latest release. There is a fork of p7zip carries (at least some) of those patches. NixOS has switched to that fork: NixOS/nixpkgs#88147

@mbauman mbauman added the security System security concerns and vulnerabilities label May 20, 2022
@StefanKarpinski
Copy link
Sponsor Member

Going forward we may want to consider dropping 7z in favor of better maintained compression libraries.

@gbaraldi
Copy link
Member

There is an upstream 7zip from the author of the windows lib now, we could use that. It seems to be regularly updated. https://www.7-zip.org/download.html

@giordano
Copy link
Contributor

But 7zip is different from p7zip, no?

@gbaraldi
Copy link
Member

From what I understand p7zip = posix7zip. And it's built on 7zip code, just old code.

@fxcoudert
Copy link
Contributor

I've studied that issue to some extent, so here are my notes on the situation:

  • p7zip was created as a fork of the old Windows-only 7zip, to make the codebase POSIX-compliant
  • p7zip was basically abandonned
  • there are various forks; the one that seems the best maintained and has been adopted by different vendors is https://github.com/jinfeihan57/p7zip (we ship it in Homebrew)
  • recently, the author of 7zip has been making steps towards making his code POSIX-compliant, and the latest version should be compatible with Linux and macOS
  • the 7zip maintainer stated that he does not use linux/unix, and basically has no way of wide testing
  • the maintainer of the p7zip fork would like to get their codebase merged back into 7zip, but does not think the current 7zip codebase is ready yet, in terms of portability

Using 7zip might be good, but I don't think it has guarantees to support all the platforms that julia does (freebsd, for example). My 20 cents of advice would be to migrate to the p7zip fork for security reasons, in the short term.

PS: For p7zip, do you need a library or just the binary?

@giordano
Copy link
Contributor

p7zip was created as a fork of the old Windows-only 7zip, to make the codebase POSIX-compliant

OK, I knew that 7zip was Windows-only and as such not really practical for us. But I see now on the website that there are binaries for Linux and macOS, I missed the progress about this.

PS: For p7zip, do you need a library or just the binary?

I think we currently use only the executable, although using a library for in-process work may not be a bad idea? Well, I wish using libraries was better than external executables, but libgit2 is a notable exception...

@ViralBShah
Copy link
Member

@fxcoudert Would the one that homebrew ships address all the CVEs? If so, it may be best for us to shift to that for now, and figure out a better solution long term.

@giordano
Copy link
Contributor

That's the same fork mentioned in the original post as being used by NixOS, and Matt said that it addresses only some of the vulnerabilities.

@mbauman
Copy link
Sponsor Member Author

mbauman commented May 23, 2022

Matt said that it addresses only some of the vulnerabilities.

I just hadn't gone through all the CVEs and matched them against patches when I started this issue. Doing so now, it looks like the only one missing is the most recent CVE-2022-29072... but that's now officially flagged as disputed. Many reputable sources are saying it's invalid. E.g., in Tom's Hardware's update on the CVE:

Update 4/20/2022 7:50amPT: The listed 7zip CVE-2022-29072 vulnerability has now been marked as "disputed" in the official listing, and "multiple third parties have reported that no privilege escalation can occur." According to Google Project Zero vulnerability researcher Tavis Ormandy who alerted us to the dispute, this exploit could only occur by editing the registry and possibly other maneuvers (like adding another Local Administrator account). However, the description isn't clear enough to discern the method of attack. We'll keep you updated if the dispute is granted.

@fxcoudert
Copy link
Contributor

CVE-2022-29072 is disputed, and it concerns the interaction between the 7zip windows executable and its help file. There is no such CHM file in p7zip, so it is even unclear at this stage if this potential vulnerability even affects p7zip.

@StefanKarpinski
Copy link
Sponsor Member

StefanKarpinski commented May 26, 2022

The only thing we currently use p7z for is gzip compression and decompression, which would, honestly be much easier to do with gzip itself than it is to coax p7z into doing it. It would probably be faster as well as more secure. We used to also use it to pack and unpack tarballs, but we use the Tar stdlib for that now. Of course for 1.6 I don't think we can reasonably drop p7z, but for future releases, we absolutely should.

@giordano
Copy link
Contributor

Pkg is supposed to support more compression formats than gzip: https://github.com/JuliaLang/Pkg.jl/blob/40caa25ee7821a03681a4f4d8ef74b40d90fa366/src/PlatformEngines.jl#L475 and it'd be nice to use more efficient compression formats for some artifacts.

@KristofferC
Copy link
Sponsor Member

I think this is closed by #45435

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

No branches or pull requests

7 participants