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

qtwebengine: add patch for CVE-2019-13720 #72794

Merged
merged 1 commit into from Nov 9, 2019
Merged

qtwebengine: add patch for CVE-2019-13720 #72794

merged 1 commit into from Nov 9, 2019

Conversation

ghost
Copy link

@ghost ghost commented Nov 4, 2019 •
edited by ghost

Motivation for this change
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 qt maintainers @qknight @ttuegel @periklis @bkchr
CC security team @grahamc @fpletz @domenkozar

@ghost ghost requested a review from ttuegel as a code owner November 4, 2019 20:37
@ghost
Copy link
Author

ghost commented Nov 4, 2019

Whoops, this patch does not apply to the source as it is

@ghost
Copy link
Author

ghost commented Nov 4, 2019

I think it should be okay now, maybe someone else can look into patches for older versions of qtwebengine, but I think this one is most important.

@ghost
Copy link
Author

ghost commented Nov 4, 2019

Meh, still broken: Does fetchpatch provide an option to remove the directory prefix (a/b) after adding the extraPrefix?

@veprbl
Copy link
Member

veprbl commented Nov 4, 2019

Meh, still broken: Does fetchpatch provide an option to remove the directory prefix (a/b) after adding the extraPrefix?

stripLen = 1 should work, just don't forget to update sha256.

@ghost
Copy link
Author

ghost commented Nov 4, 2019

Ah, got it. Thanks for the help.

@d-goldin
Copy link
Contributor

d-goldin commented Nov 8, 2019

nix-review report:

[117 built (3 failed), 1323 copied (9869.6 MiB), 2226.9 MiB DL]
error: build of '/nix/store/dc4v7ah4js6ilsf5xdp9bgqxgl2mj74c-env.drv' failed
https://github.com/NixOS/nixpkgs/pull/72794
1 package are marked as broken and were skipped:
gopherclient

4 package failed to build:
spyder python38Packages.pyside2 python38Packages.pyside2-tools python38Packages.spyder

104 package were build:
akregator amarok anki calamares clipgrab digikam eagle falkon fcitx-engines.libpinyin freecad ghostwriter kaddressbook kde-cli-tools kdeApplications.akonadi-calendar kdeApplications.akonadi-contacts kdeApplications.akonadi-import-wizard kdeApplications.akonadiconsole kdeApplications.calendarsupport kdeApplications.eventviews kdeApplications.incidenceeditor kdeApplications.kalarm kdeApplications.kdepim-addons kdeApplications.kdepim-apps-libs kdeApplications.kdepim-runtime kgpg kmail kdeApplications.kmail-account-wizard kdeApplications.kmailtransport kdeApplications.knotes kdeApplications.konqueror kontact korganizer kdeApplications.libgravatar kdeApplications.libkdepim kdeApplications.libkgapi kdeApplications.libksieve kdeApplications.mailcommon kdeApplications.mailimporter kdeApplications.mbox-importer kdeApplications.messagelib kdeApplications.pim-data-exporter kdeApplications.pim-sieve-editor kdeApplications.pimcommon kdeplasma-addons kdev-php kdev-python kdevelop kdevelop-unwrapped kmenuedit ksysguard kwin-tiling plasma5.khotkeys plasma5.libksysguard luminanceHDR luxcorerender mendeley minizincide monero-gui multibootusb musescore nextcloud-client nmapsi4 otter-browser plasma-desktop plasma-vault plasma-workspace powerdevil systemsettings psi-plus python27Packages.foxdot python27Packages.pyqtwebengine python27Packages.pyside2 python27Packages.pyside2-tools python37Packages.foxdot python37Packages.pyqtwebengine python37Packages.pyside2 python37Packages.pyside2-tools python38Packages.foxdot python38Packages.pyqtwebengine qmapshack qolibri qsyncthingtray qt5Full qt5.qtwebengine qt5.qtwebview qutebrowser radare2-cutter rssguard rstudio rstudioWrapper seafile-client skrooge sonic-pi supercollider supercollider_scel syncthingtray teamspeak_client toggldesktop trojita vnote wacomtablet webmacs zanshin zoom-us

The spyder failure is pre-existing in current master.

Aside from that - is there an alternative link/resource we could provide in the comment? The currently provided one requires a login.

@ghost
Copy link
Author

ghost commented Nov 9, 2019

Thanks for running nix-review.
These are some other resources I could find, but I'm not sure which should be linked:
https://www.mail-archive.com/qutebrowser@lists.qutebrowser.org/msg00645.html
https://git.archlinux.org/svntogit/packages.git/commit/trunk?h=packages/qt5-webengine&id=3946bcd53cc07b3c56a9ac0409cc748c5fa0ad4c

I think it is critical that we can ship this soon, since it is a remote arbitrary code execution in browsers that some of us use being exploited in the wild. The maintainers or members of the security team can push changes to my branch as they see fit.

@d-goldin
Copy link
Contributor

d-goldin commented Nov 9, 2019

Let's ping @globin @andir @worldofpeace additionally.

@globin globin merged commit cdf9b62 into NixOS:master Nov 9, 2019
@globin
Copy link
Member

globin commented Nov 9, 2019

Thanks, also testing backports

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ttuegel ttuegel Awaiting requested review from ttuegel ttuegel is a code owner

Assignees
No one assigned
Labels
1.severity: security 6.topic: qt/kde 10.rebuild-darwin: 1-10 10.rebuild-linux: 101-500
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@veprbl @d-goldin @globin