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.

Sign up for GitHub

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

Jump to bottom

libxml2: 2.9.9 -> 2.9.10, libxslt: 1.1.33 -> 1.1.34, addressing CVE-2019-18197 #73178

Merged
merged 2 commits into from Nov 13, 2019
Merged

libxml2: 2.9.9 -> 2.9.10, libxslt: 1.1.33 -> 1.1.34, addressing CVE-2019-18197 #73178

merged 2 commits into from Nov 13, 2019

Conversation

risicle
Copy link
Contributor

@risicle risicle commented Nov 10, 2019 •
edited

Motivation for this change

https://nvd.nist.gov/vuln/detail/CVE-2019-18197

libxslt 1.1.34 requires libxml2 2.9.10. To get libxml2 2.9.10 building successfully I had to disable a couple of the python tests. These tests had actually always been failing on previous versions with python >= 3.5 - the results had just been previously ignored by the makefiles. We're also not the only ones to have had this problem: https://mail.gnome.org/archives/xml/2017-August/msg00014.html

I also checked the new libxslt does indeed include the fixes for the patches I removed in the bump.

Anyway, I clearly have not done a full rebuild - I OOM'd my laptop just trying to compute the rebuilds.

Supersedes #72549

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

@ofborg ofborg bot requested a review from edolstra November 10, 2019 21:30
@flokli flokli mentioned this pull request Nov 11, 2019
Closed
flokli
flokli reviewed Nov 11, 2019
View reviewed changes
Copy link
Contributor

@flokli flokli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some nitpicks, apart from that, LGTM.

pkgs/development/libraries/libxml2/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libxml2/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libxml2/default.nix
# disable test that's problematic with newer pythons: see
# https://mail.gnome.org/archives/xml/2017-August/msg00014.html
preCheck = if pythonSupport && !(python?pythonOlder && python.pythonOlder "3.5") then ''
echo "" > python/tests/tstLastError.py
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't we just

Suggested change
echo "" > python/tests/tstLastError.py
rm python/tests/tstLastError.py

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The makefile isn't clever enough to cope with a missing file and still tries to call it in this case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would this failure suddenly occur for older Python versions while it did not in our case before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because, as I said, the failure did indeed happen (look at hydra's logs), the makefile just never paid attention to its exit code. This changed in GNOME/libxml2@d188eb9 so it suddenly became a problem for us. I did look into it for a while, but to no avail - looks like it's deep in c module code.

@risicle
libxml2: 2.9.9 -> 2.9.10
c3c77ec 
disable python test which was previously failing anyway, but in previous
versions it was being ignored
@risicle
libxslt: 1.1.33 -> 1.1.34 (security)
4fec9d5 
addresses CVE-2019-18197
FRidh
FRidh reviewed Nov 11, 2019
View reviewed changes
pkgs/development/libraries/libxml2/default.nix
@@ -53,6 +53,12 @@ stdenv.mkDerivation rec {

enableParallelBuilding = true;

# disable test that's problematic with newer pythons: see
# https://mail.gnome.org/archives/xml/2017-August/msg00014.html
preCheck = lib.optionalString (pythonSupport && !(python?pythonOlder && python.pythonOlder "3.5")) ''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

python?pythonOlder did you notice an interpreter that did not have this? A common passthru is used for all Python interpreters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meh I was just fitting in with lower down where python?isPy3 is used.

@FRidh FRidh added this to WIP in Staging via automation Nov 11, 2019
@risicle risicle mentioned this pull request Nov 11, 2019
Merged
10 tasks
@risicle
Copy link
Contributor Author

risicle commented Nov 11, 2019

@GrahamcOfBorg build libxml2 libxslt xmlstarlet python37Packages.lxml

@flokli flokli merged commit abdc1ea into NixOS:staging Nov 13, 2019
Staging automation moved this from WIP to Done Nov 13, 2019
@vcunat vcunat mentioned this pull request Dec 29, 2019
Closed
1 task
@markuskowa markuskowa mentioned this pull request Jan 9, 2020
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flokli flokli flokli left review comments

@FRidh FRidh FRidh left review comments

@edolstra edolstra Awaiting requested review from edolstra

Assignees
No one assigned
Labels
1.severity: security 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-darwin-stdenv 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
Staging
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@risicle @flokli @FRidh @veprbl