After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 657374 - mailto: attachment parameter can lead to accidental data exfiltration
mailto: attachment parameter can lead to accidental data exfiltration
Status: RESOLVED FIXED
Product: evolution
Classification: Applications
Component: Mailer
unspecified
Other Linux
: Normal critical
: ---
Assigned To: evolution-mail-maintainers
Evolution QA team
Depends on:
Blocks:
 
 
Reported: 2011-08-25 20:00 UTC by Matt McCutchen
Modified: 2011-11-30 13:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch for Evolution (2.15 KB, patch)
2011-09-13 13:06 UTC, Vibha Yadav
needs-work Details | Review
Patch for Evolution (1.68 KB, patch)
2011-09-13 13:16 UTC, Vibha Yadav
needs-work Details | Review
Patch for Evolution (2.36 KB, patch)
2011-09-14 12:14 UTC, Vibha Yadav
committed Details | Review
Patch for Evolution (1.89 KB, patch)
2011-09-14 12:26 UTC, Vibha Yadav
none Details | Review

Description Matt McCutchen 2011-08-25 20:00:02 UTC
By registering a mailto: URL handler that allows local files to be attached via a query parameter, you are letting arbitrary web sites place the user one unsuspecting click away from exfiltrating confidential data.  E.g.:

https://mattmccutchen.net/private/evolution-mailto-test

I'm not sure if the security model for URL handlers has been decided in general, but given that browsers currently allow untrusted sites to call the mailto: handler, it needs to have appropriate safeguards.  I would suggest showing a prompt with a checkbox, initially off, for each attachment specified in the URL.
Comment 1 Matthew Barnes 2011-08-25 20:42:39 UTC
I think this needs to be handled by web browsers.  Nautilus SendTo, for example, invokes Evolution with a mailto: URL and I don't want to get prompted with a scary warning about attaching a file that I just told Nautilus to attach to an email.
Comment 2 Matt McCutchen 2011-08-25 20:43:42 UTC
Oh... I didn't anticipate that you would want this private and already filed a public Fedora bug (https://bugzilla.redhat.com/show_bug.cgi?id=733504), which I cannot now make private.
Comment 3 Matt McCutchen 2011-08-25 20:59:01 UTC
(In reply to comment #1)
> I think this needs to be handled by web browsers.

I think you mean all applications that present untrusted links, which also includes IM clients and HTML mail readers such as Evolution.  So your suggestion would not get you out of changing Evolution.

After the URL handler security model has been ambiguous for so long, assuming the URLs to be trustworthy or sanitized and obliging all callers to comply is unrealistic, especially with respect to an Evolution-specific query parameter.

> Nautilus SendTo, for
> example, invokes Evolution with a mailto: URL and I don't want to get prompted
> with a scary warning about attaching a file that I just told Nautilus to attach
> to an email.

So we introduce a different API for that.  At any rate, it is a lesser evil than this bug.
Comment 4 Vincent Untz 2011-09-06 16:02:58 UTC
Since the Red Hat bug is public and cannot be made private, is it okay to make this bug public too?
Comment 5 Matthew Barnes 2011-09-06 16:23:12 UTC
Bug is public now.
Comment 6 Vibha Yadav 2011-09-13 13:06:58 UTC
Created attachment 196367 [details] [review]
Patch for Evolution

Added the code for providing the warning in if mailto command is trying to attach any blacklisted file.

The blacklist is currently:

- hidden files (e.g. ".foo").
- files in hidden directories (e.g. ".secret/foo").
- files under /etc.
- files with ".." as a path component.
Comment 7 Vibha Yadav 2011-09-13 13:16:22 UTC
Created attachment 196368 [details] [review]
Patch for Evolution

Patch for corporate releases.
Here instead of providing a warning in composer window. Blocking the blacklisted file from getting attached and providing a warning on terminal.

Backported to 2.28
Comment 8 Matthew Barnes 2011-09-13 14:59:56 UTC
Review of attachment 196367 [details] [review]:

A simple substring search against the blacklist is too simplistic.  You're going to exclude perfectly safe filenames like "README.txt" ('.' rule).

The filename needs to be broken into path components using

   g_strsplit (filename, G_DIR_SEPARATOR_S, -1)

and then compare each path component against the blacklist using prefix match (g_str_has_prefix()) instead of a substring match.

Also I think the wording of the alert could be improved.  Perhaps something like:

   The mailto: URL included a suspicious looking attachment

   A suspicious looking file named "$FILENAME" was removed as a precaution.
   You can reattach this file using the "Add Attachment" button below.
Comment 9 Matthew Barnes 2011-09-13 15:05:38 UTC
Review of attachment 196368 [details] [review]:

Same issue here with the blacklist comparison.  You're correct to just use a terminal message here to avoid new translatable strings.
Comment 10 André Klapper 2011-09-13 15:24:02 UTC
Why blacklist at all? Can't I get always in case that the &attachment parameter was passed (in case this can be identified and that the filename[s] can be extracted) a popup telling me that $filename was automatically attached and that I should make sure that I really want to send this file?
Comment 11 Matthew Barnes 2011-09-13 15:56:54 UTC
Such a dialog would be annoying and redundant if the attachment was submitted intentionally by way of Nautilus-Sendto or even a command-line or shell script invocation.
Comment 12 Matt McCutchen 2011-09-13 19:01:08 UTC
(In reply to comment #11)
> Such a dialog would be annoying and redundant if the attachment was submitted
> intentionally by way of Nautilus-Sendto or even a command-line or shell script
> invocation.

See comment #3.
Comment 13 Vibha Yadav 2011-09-14 12:14:16 UTC
Created attachment 196487 [details] [review]
Patch for Evolution

Rework on review comments.
Comment 14 Vibha Yadav 2011-09-14 12:26:04 UTC
Created attachment 196488 [details] [review]
Patch for Evolution

Reworked on review comments for 2.28
Comment 15 André Klapper 2011-09-14 12:39:12 UTC
> g_warning("Mailto command is trying to attach blacklisted file: %s. The
> attachment may contain sensitive data. Kindly review it", content);

* "Trying"? So it's not done and I still have to attach it manually 
  afterwards if this was indeed wanted?
* Missing fullstop at the end
* If e.g. nautilus-sendto triggered this, no user necessarily needs 
  to know what a "mailto command" is...
* "Please review it before sending." maybe?
Comment 16 Vibha Yadav 2011-09-15 11:58:14 UTC
> * "Trying"? So it's not done and I still have to attach it manually 
>   afterwards if this was indeed wanted?

Yes it is not yet attached in case of 2.28.
> * Missing fullstop at the end
> * If e.g. nautilus-sendto triggered this, no user necessarily needs 
>   to know what a "mailto command" is...
> * "Please review it before sending." maybe?

Making it following message for 2.28.

g_warning("Blacklisted file: %s is trying to get attached. The attachment may contain sensitive data. Please review it before sending.", content); 

and following for master

+  <error id="blacklisted-file" type="warning">
+    <_primary>A blacklisted file is attached.</_primary>
+    <_secondary xml:space="preserve">The attachment named {0} is a hidden file and may contain sensitive data. Please review it before sending.</_secondary>
+  </error>
+
Comment 17 André Klapper 2011-09-15 12:06:05 UTC
Still I wonder if the user really needs to know anything about "blacklisting" here... sounds scary.
Comment 18 Matthew Barnes 2011-09-15 12:26:51 UTC
Review of attachment 196487 [details] [review]:

Okay, the blacklist comparison routine looks good now.  I still like my suggested wording for the alert text better (comment #8).
Comment 19 Matthew Barnes 2011-09-15 12:29:00 UTC
Review of attachment 196488 [details] [review]:

Same as comment #18.  I'll let you and Andre duke it out over the wording, but a terminal message is less important since it isn't translated.
Comment 20 Matt McCutchen 2011-09-16 03:04:17 UTC
A blacklist is not a solution.  I have plenty of confidential files at paths that have no hidden component and contain no randomness; it's too strong an assumption that such paths will never be guessed or accidentally leaked by an application or in a log posted to a bug report, etc.  Attachments specified by untrusted mailto URLs should not be honored without explicit user consent.

There can be an additional mimeapp for "x-scheme-handler-trusted/mailto" or so which Nautilus-Sendto would use and which would include a flag that tells Evolution to accept the attachments without prompting.
Comment 21 Matthew Barnes 2011-09-16 03:49:14 UTC
It's good enough for now.
Comment 22 Matt McCutchen 2011-09-16 23:29:06 UTC
BS.  Looks like I'll be carrying a patch.
Comment 23 Vibha Yadav 2011-10-04 06:42:15 UTC
Comment on attachment 196487 [details] [review]
Patch for Evolution

committed to master in 273b10a
Comment 24 André Klapper 2011-11-04 15:42:38 UTC
(In reply to comment #23)
> (From update of attachment 196487 [details] [review])
> committed to master in 273b10a

So can this be closed as FIXED? Or does anybody plan to backport something to 3.2 that does not introduce new strings?
Comment 25 Milan Crha 2011-11-28 09:01:15 UTC
I'm marking it as fixed, the bug is sort of lame anyway, because you see a list of attachments being attached, before the message is sent.
Comment 26 Matthew Barnes 2011-11-30 13:26:14 UTC
Backported to 3.2, which just dumps a message to the terminal and silently skips suspicious looking mailto attachments which match the blacklist:

http://git.gnome.org/browse/evolution/commit/?h=gnome-3-2&id=588c410718068388f8ce0004a71c104a4c89cce3