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

Jump to bottom

libsass: 3.6.1 -> 3.6.3 #82377

Merged
merged 1 commit into from Apr 5, 2020
Merged

libsass: 3.6.1 -> 3.6.3 #82377

merged 1 commit into from Apr 5, 2020

Conversation

danderson
Copy link
Contributor

Motivation for this change

Contains fix for CVE-2019-18798 and CVE-2019-18799.

Fixes #54804, #53571, #57155, #58266, #73660.

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 nixpkgs-review --run "nixpkgs-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.

@danderson
Copy link
Contributor Author

@grahamc for security.

@ofborg ofborg bot requested review from offlinehacker and codyopel March 12, 2020 02:59
@danderson
Copy link
Contributor Author

Surprised at the reverse dependency size, but nix-review confirms. I guess ~everything with a web UI uses sass these days.

But hey, CVE, so yay mass rebuild :(

@marsam
Copy link
Contributor

marsam commented Mar 12, 2020

would you mind changing the base branch to staging?

@danderson
Copy link
Contributor Author

Done, transplanted to staging.

@alyssais alyssais changed the base branch from master to staging March 12, 2020 06:59
@alyssais
Copy link
Member

@danderson you need to change the base branch in the GitHub UI as well as rebasing. (I’ve done it for you this time.)

@alyssais
Copy link
Member

fwiw, we have previously not taken this update because of some regressions it introduces. See #72934. Although I don’t think we know about the security issues then.

So I guess there’s a trade off to be made here. Fix the CVEs and break packages, or don’t? Cc @FRidh @hedning @lazka.

Maybe we could keep 3.6.1, but just apply the fixes for the CVEs?

@hedning
Copy link
Contributor

hedning commented Mar 13, 2020

Right, given that this is fixed in gtk, and it's hard to backport the needed fixes, it probably makes sense to merge this as is, and deal with any fallout. Breaking a few themes isn't that big of a deal.

I guess the main issue could be broken builds on hydra which consumes too much resources. So we should watch out for that and mark offending derivations as broken.

@alyssais
Copy link
Member

@danderson please drop the revert commit and we can merge

@danderson
Copy link
Contributor Author

Oops, how did that get pushed. Sorry! Fixed.

@alyssais
Copy link
Member

@GrahamcOfBorg build arc-theme

@alyssais
Copy link
Member

Somebody probably needs to do a nix-review and mark broken any packages that OOM with a reasonable amount of memory

@danderson
Copy link
Contributor Author

Ack. I don't have a machine with the resources to nix-review this right now. Will attempt to locate one and do this.

@FRidh FRidh merged commit 92ffd64 into NixOS:staging Apr 5, 2020
Staging automation moved this from Needs review to Done Apr 5, 2020
@FRidh
Copy link
Member

FRidh commented Apr 5, 2020

Checking themes can be done while its in staging-next so we have a stdenv.

@int-index
Copy link
Contributor

Building arc-theme with this patch ate my RAM (16GiB), froze the system, and I had to hard reset. This broke my file system and I had to rescue it by booting up a USB stick and doing fsck.

Then I added 16GiB of swap and tried again. It ate my swap. I had to revert this patch locally and use libsass-3.6.1.

Maybe we should mark arc-theme as broken for now?

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/out-of-memory-rebuilding-nixos-unstable/6786/3

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/my-laptop-froze-mid-upgrade-and-now-im-getting-strange-errors/6781/3

@danderson danderson deleted the bug-54804 branch April 21, 2020 03:49
@FRidh
Copy link
Member

FRidh commented Apr 21, 2020

marked arc-theme and yaru-theme as broken on master.

@turboMaCk
Copy link
Member

turboMaCk commented Apr 21, 2020 •
edited

I think this also most likely broke hlibsass (haskell bindings to libsass) and all the packages downstream.

I was wrong. This actually fixes it. I had version of channel on my machine.

@danderson danderson restored the bug-54804 branch November 4, 2020 03:50
@chkno chkno mentioned this pull request Nov 30, 2020
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@offlinehacker offlinehacker Awaiting requested review from offlinehacker

@codyopel codyopel Awaiting requested review from codyopel

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

Assignees
No one assigned
Labels
8.has: clean-up 10.rebuild-darwin: 501-1000 10.rebuild-darwin: 501+ 10.rebuild-linux: 501+ 10.rebuild-linux: 2501-5000
Projects
Staging
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Vulnerability roundup 60: libsass-3.5.5: 3 advisories
9 participants
@danderson @marsam @alyssais @lazka @hedning @FRidh @int-index @nixos-discourse @turboMaCk