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
icedtea_web: 1.7.1 -> 1.7.2 (plus CVE patches) #66444
icedtea_web: 1.7.1 -> 1.7.2 (plus CVE patches) #66444
Conversation
I believe this is because Also, is it possible these binary blobs were important? If doing that proves too difficult, maybe we can upload it somewhere else. |
@worldofpeace the patch comes straight from AdoptOpenJDK/IcedTea-Web#346 (i.e. https://patch-diff.githubusercontent.com/raw/AdoptOpenJDK/IcedTea-Web/pull/346.patch) The binary blobs are
They are used in tests, the Changelog in the diff has some more information. I don't think that the nix package has ever run all tests, so I think it should be OK to not have them in the scope of this PR. Looking forward to hearing from you. Thanks |
Ok,
Ok, that clears up "Also, is it possible these binary blobs were important?" but I still think we shouldn't have such a large patch in nixpkgs for practical reasons. Do you disagree @grahamc? |
I'm not sure I understand. I have removed the blobs from the patch, so it shouldn't be too big. It should include just the code changes. $ du -sh pkgs/development/compilers/icedtea-web/cve-fixes.patch
48K pkgs/development/compilers/icedtea-web/cve-fixes.patch Am I missing something? |
Yes, we have a guideline to try our best to fetch patches from remote places and not keep them in nixpkgs. I personally support this since it keeps nixpkgs much smaller. |
I see! Thanks for the explanation. I'm happy to rework this to include the patch remotely then. |
I removed the patch file and use The package builds fine locally and the executables continue to work. |
Can we remove |
|
There's a distinct change in executable names Before
After
I'd say that's backwards incompatible. |
@worldofpeace are you sure about that? I get the following:
The executable names change in version 1.8.x, but should stay the same in 1.7.2 |
Building 65729e0 I get
On 7a859977bcdc3461bf95129612a7945bb452c8c2
Perhaps something's up with my setup, could you build those two revisions and see the diff? |
I see! My change definitely builds Thanks for spotting that! |
I checked and the executables are all shell scripts. I will remove the extension and filter out |
Removed Note that we need to keep Re-tested manually and working as expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still works, thanks for fixing this 👍
On Wed, 31 Jul 2019 it was announced that IcedTea-Web was affected by the below security vulnerabilities: - CVE-2019-10185: zip-slip attack during auto-extraction of a JAR file. - CVE-2019-10181: executable code could be injected in a JAR file without compromising the signature verification. - CVE-2019-10182: improper path sanitization from elements in JNLP files. Version 1.7 was patched, but no release was made. Moreover, the patches apply cleanly only to 1.7.2, not the current 1.7.1. Rather than marking 1.7.1 as insecure, update to 1.7.2 and apply the official patches. References: https://www.openwall.com/lists/oss-security/2019/07/31/2 AdoptOpenJDK/IcedTea-Web#327 AdoptOpenJDK/IcedTea-Web#346
gtk2 is not needed any more
icedtea-web 1.7.2 builds its launchers shell scripts with the "sh" extension, while version 1.7.1 did not. For backwards-compatibility, remove the extension from the executable in postInstall. Note that version 1.7.2 also creates a file called itw-modularjdk.args in the bin directory. This file is referenced by the shell launchers, so we leave it there (it's not executable anyway).
On Wed, 31 Jul 2019 it was announced that IcedTea-Web was affected by the below
security vulnerabilities:
CVE-2019-10185: zip-slip attack during auto-extraction of a JAR file.
CVE-2019-10181: executable code could be injected in a JAR file without
compromising the signature verification.
CVE-2019-10182: improper path sanitization from elements in JNLP
files.
Version 1.7 was patched, but no release was made. Moreover, the patches apply
cleanly only to 1.7.2, not the current 1.7.1.
Rather than marking 1.7.1 as insecure, update to 1.7.2 and apply the official
patches.
Note that the patch file has been edited to remove the addition of binaryblobs (that cannot be applied).
References:
https://www.openwall.com/lists/oss-security/2019/07/31/2
AdoptOpenJDK/IcedTea-Web#327
AdoptOpenJDK/IcedTea-Web#346
Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Same manual testing as done in #66422 but using the shell launchers instead of the rust ones.
Notify maintainers
cc @grahamc (NixOS security team)
@worldofpeace (reviewer of related #66422)