"Reply with Template" Filters leak email address to mailing lists

RESOLVED FIXED in Thunderbird 45.0

Status

defect
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: dveditz, Assigned: mkmelin)

Tracking

(Blocks 2 bugs, {privacy})

unspecified
Thunderbird 45.0
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Reporter

Description

6 years ago
Apparently some people use the "Reply with Template" filter feature to create a manual vacation responder. I guess they leave Thunderbird running while they're on vacation? We've received complaints that this violates RFC 3834 and can leak the users personal email back to members of a list who otherwise may not know that personal address.

> <http://tools.ietf.org/html/rfc3834>
> 
> Personal and Group responses whose purpose is to notify the sender of
> a message of a temporary absence of the recipient (e.g., "vacation"
> and "out of the office" notices) SHOULD NOT be issued unless a valid
> address for the recipient is explicitly included in a recipient
> (e.g., To, Cc, Bcc, Resent-To, Resent-Cc, or Resent- Bcc) field of
> the subject message.

First, that's not "MUST NOT" so we're not violating that RFC, but it's a valid concern. On the other hand message filters are not intrinsically a vacation responder feature, and they're missing some important refinements to be one (such as the ability to only send one reply per address, and ease of user).

One way to address this would be that if the user chooses the "Reply with Template" filter action we automatically add a rule of

   To or CC     Contains        <account address>

This would not be a hidden implicit rule, it would be added to the visible list so that a user could adjust it or delete it if it's inappropriate.
Assignee

Updated

6 years ago
OS: Windows 7 → All
Hardware: x86_64 → All
Assignee

Comment 1

6 years ago
Posted patch proposed fix (obsolete) — Splinter Review
Don't do auto-reply if we're not in To/Cc. (I don't see the point for Bcc, and the other headers suggested aren't easily available i think.)

I'm not sure what should be done about filter logging for cases like these. Currently logging is done before action execution http://mxr.mozilla.org/comm-central/source/mailnews/base/search/src/nsMsgFilterService.cpp#465
On the other hand, the filter was executed, it just refused to actually do anything.
Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #798314 - Flags: review?(kent)
Assignee

Updated

6 years ago
Blocks: 303310
Assignee

Comment 2

6 years ago
Posted patch proposed fix, v2 (obsolete) — Splinter Review
I think the previous version wasn't what i tested...
Attachment #798314 - Attachment is obsolete: true
Attachment #798314 - Flags: review?(kent)
Attachment #799025 - Flags: review?(kent)
Assignee

Comment 3

5 years ago
Rebased to apply on top of bug 904458 + added/adjusted test.

Kent, if you don't think you'll get to this anytime soon, can you redirect the review to irving perhaps?
Attachment #799025 - Attachment is obsolete: true
Attachment #799025 - Flags: review?(kent)
Attachment #8408920 - Flags: review?(kent)
Somehow I thought this review request went away, but it seems to be back again. I'll look at it.
Assignee

Comment 5

5 years ago
Note that bug 904458 got backed out due to some mysterious crash.
(This bug might also be better using some own test data, instead of reusing bugmail11, I've not yet verified changing the To doesn't break other tests using it.)
Comment on attachment 8408920 [details] [diff] [review]
bug904478_reply_with_template_replies_to_mailing_lists.patch

Review of attachment 8408920 [details] [diff] [review]:
-----------------------------------------------------------------

I'm happy with the main code (with one minor issue) - but not with the tests. But I have not been able myself to come up with a version of the tests that will work.

What I think we should do with this, and also with the predecessor bug 904458, is to land it without the tests, and add a separate bug to resolve the test issues. I don't think we should hold these bugs hostage to issues with the testing framework, which is what we seem to be dealing with.

Though I am still investigating this, what I think i will do is r+ with two changes - fix the missing rv nit, and remove the tests.

::: mailnews/compose/src/nsMsgComposeService.cpp
@@ +1086,5 @@
>    // I think we're going to need to stream the template message to ourselves,
>    // and construct the body, and call setBody on the compFields.
> +  nsresult rv;
> +  nsCOMPtr <nsIMsgAccountManager> accountManager =
> +    do_GetService(NS_MSGACCOUNTMANAGER_CONTRACTID);

You need do_GetService(NS_MSGACCOUNTMANAGER_CONTRACTID, &rv) otherwise rv is undefined.
Attachment #8408920 - Flags: review?(kent) → review+
Assignee

Comment 7

5 years ago
So, how about we land the tests but skip running them on osx and linux debug since the others are fine? Having them landed makes it much easier too to keep track of it all, and for local testing. Probably

skip-if = ( os == "mac") || (os == "linux" && debug)
I would be OK with that - but I think then you need to change the tests to use only a single framework (that is get rid of async_test_utils) and also use add_test instead of add_task. (Though I am not an expert on the newer framework, by understanding is that add_task expects to yield a promise).

See some of the changes that I made in https://hg.mozilla.org/try-comm-central/rev/7010fcd736e0
Assignee

Comment 9

4 years ago
Test now using just one async framework
Attachment #8408920 - Attachment is obsolete: true
Attachment #8697086 - Flags: review?(rkent)
Dependent bug's patch needed for the test_autoReply.js file
Depends on: 904458
Comment on attachment 8697086 [details] [diff] [review]
bug904478_reply_with_template_replies_to_mailing_lists.patch

Review of attachment 8697086 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM.
Attachment #8697086 - Flags: review?(rkent) → review+
Assignee

Comment 12

4 years ago
https://hg.mozilla.org/comm-central/rev/79e74959cca2 -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 45.0

Updated

2 years ago
See Also: → 1334980
You need to log in before you can comment on or make changes to this bug.