search

JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.68k stars 5.48k forks source link

Security issues with p7zip #45397

Closed mbauman closed 2 years ago

mbauman commented 2 years ago

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: https://github.com/NixOS/nixpkgs/issues/88147

StefanKarpinski commented 2 years ago

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

gbaraldi commented 2 years ago

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 commented 2 years ago

But 7zip is different from p7zip, no?

gbaraldi commented 2 years ago

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

fxcoudert commented 2 years ago

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

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

I think this is closed by https://github.com/JuliaLang/julia/pull/45435