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
gettext: apply patch for CVE 2018-18751 #58997
Conversation
This security update was under the radar, because the CVE wasn't formatted correctly. 0.19.8.1 is still affected (and the package hasn't seen a release since ages) but the CVE only talks about 0.19.8. I'd also like to get a backport to 18.09 if possible, thanks! |
Thank you for looking into this! 👍 I usually prefer if you just open a backport PR yourself. It ensures we have the same level of checks / eval / compile / ofBorg / … on those as well. Stable branches should receive that level of diligence :-) I am reviewing the changes themselves now. |
Alright, sure thing! I'll get another one out in a minute. |
I am tempted to have this go into the respective staging branches first since the amount of rebuilds is quiet large and the issue only occurs when used as a commandline tool. |
Yeah, it's a package that's used in the most basic build environment and thus causes massive rebuilds. Do I need to do anything wrt to directing this to the staging branch? |
Alright, I targeted the staging branch for the backport. That's #59000 |
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.
Besides moving this to staging and the few comments below there is nothing I see that is wrong about this.
(Sorry hit the wrong button there while constructing a comment) To move your changes to https://help.github.com/en/articles/changing-the-base-branch-of-a-pull-request shows how to retarget to another branch. |
4915da4
to
a121681
Compare
Alright, base branch is changed and the patch isn't vendored any longer. The newline is still there because I'd like to understand whether that's really the policy :) |
a121681
to
8cca54f
Compare
Alright, requested changes are in here. Let me know if you're happy and I'll update the stable PR. |
When trying to build
|
Interesting, what's the command you're using? I did nix-build -A gettext. |
I use the very same command.
|
I'm on MacOS ➜ nixpkgs git:(gettext-security-update) nix --version
nix (Nix) 2.2 It will be built twice, right? Did this fail on the first gettext or the second? I didn't wait for all of GCC and the other environment to build so that could be it ... |
On 18.09 it is also failing and yes I still believe that it is being built twice due to bootstrapping. Darwin bootstrapping may still be different. @GrahamcOfBorg build gettext |
Re-include an older automake (1.15) because that's explicitly depended upon.
8cca54f
to
3d9e28e
Compare
Ok, so fetchPatch worked on MacOS but not Linux. I was able to reproduce the issue on NixOS and it was fixed after switching to fetchUrl. PR is up to date. |
Alright, I'll update the backport PR now. |
This reverts commit 742416a. Moved to staging.
Re-include an older automake (1.15) because that's explicitly depended upon.
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)