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

xdg-utils: fix mailto-vulnerability #95758

Closed
wants to merge 1 commit into from
Closed

Conversation

Mic92
Copy link
Member

@Mic92 Mic92 commented Aug 18, 2020

Motivation for this change

https://www.nds.ruhr-uni-bochum.de/media/nds/veroeffentlichungen/2020/08/15/mailto-paper.pdf

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.

@mweinelt
Copy link
Member

Took the liberty to update your motivation.

@SFrijters
Copy link
Member

It would be nice if you could get upstream to make a new release out of it (last one was 2 years ago) - I'm locally using an overlay to use https://gitlab.freedesktop.org/xdg/xdg-utils/-/commit/9816ebb3e6fd9f23e993b8b7fcbd56f92d9c9197 which has been on master for a couple of months now. If a new release takes too long/doesn't happen, would it be possible to point nixpkgs at a version that is not an official release?

@ofborg ofborg bot requested a review from edolstra August 19, 2020 00:42
@oxzi oxzi mentioned this pull request Aug 19, 2020
10 tasks
ajs124 pushed a commit that referenced this pull request Sep 15, 2020
The "Mailto: Me Your Secrets"[0] paper describes vulnerabilities in
multiple email clients regarding the undocumented "attach" field of a
mailto URI. This might allow the inclusion of sensitive data in an
outgoing email.

Pull request #95758 addresses this issue on a more general level.

Claws Mail unfortunately also has problems with mailto URIs[1][2].
Referring to the paper, problems for "attach" and "insert" were found
and fixed. These patches, which are not included in a release yet, are
hereby added.

[0]:https://www.nds.ruhr-uni-bochum.de/media/nds/veroeffentlichungen/2020/08/15/mailto-paper.pdf
[1]:https://www.thewildbeast.co.uk/claws-mail/bugzilla/show_bug.cgi?id=4373
[2]:https://www.thewildbeast.co.uk/claws-mail/bugzilla/show_bug.cgi?id=4374
Copy link
Member

@raboof raboof left a comment

Choose a reason for hiding this comment

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

I can reproduce the problem with chromium and the following config:

  environment.etc."xdg/mimeapps.list" = {
    text = ''
      [Default Applications]
      x-scheme-handler/mailto=thunderbird.desktop
    '';
  };

I didn't check applying this patch fixes the problem (that seems to require rebuilding chromium...), but it looks convincing.

It also breaks 'legitimate' use cases for adding attachments to emails, not only for filenames passed in via the mailto url, but also for those passed with '--attach' (since that just appends it to the mailto url AFAICS, https://gitlab.freedesktop.org/xdg/xdg-utils/-/blob/master/scripts/xdg-email.in#L372).

So we have to weigh breaking this feature against fixing the vulnerability (which is a bit far-fetched and requires user interaction) or doing nothing and leaving it to upstream to make the call.

I'd say it might be best to err on the safe side and apply this patch until we find a strong reason the feature is needed (or there is a decision upstream that we want to follow).

I looked around a bit and didn't find a CVE for this issue, btw.

@Mic92
Copy link
Member Author

Mic92 commented Oct 20, 2020

I replaced it with a pull request now.

@mweinelt
Copy link
Member

@mweinelt
Copy link
Member

mweinelt commented Nov 26, 2020

CVE-2020-27748 got assigned to this and even Debian is now considering this patch and Ubuntu just sent out an Advisory with a fixed version.

@@ -32,6 +32,14 @@ stdenv.mkDerivation rec {
# just needed when built from git
buildInputs = [ libxslt docbook_xml_dtd_412 docbook_xsl xmlto w3m ];

patches = [
# https://gitlab.freedesktop.org/xdg/xdg-utils/-/merge_requests/28
(fetchpatch {
Copy link
Member

Choose a reason for hiding this comment

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

@Mic92 please remove the trailing whitespace here

@Mic92
Copy link
Member Author

Mic92 commented Nov 26, 2020

I leave this to @mweinelt to fix this ;)

@Mic92 Mic92 closed this Nov 26, 2020
@mweinelt
Copy link
Member

Awesome, then please add a remark to the upstream merge request concerning the breakage.

@Mic92
Copy link
Member Author

Mic92 commented Nov 29, 2020

The problem is that it break the --attach flag:

$ xdg-email [--utf8] [--cc address] [--bcc address] [--subject text] [--body text
] [--attach file] [ mailto-uri | address(es) ]

@Mic92 Mic92 deleted the xdg-utils branch July 23, 2021 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants