Closed Bug 1367156 Opened 7 years ago Closed 7 years ago

Unwanted inline image shown - Make TB more tolerant to incorrect MIME structure: multipart/alternative where last part is an inline image

Categories

(MailNews Core :: MIME, defect)

defect
Not set
normal

Tracking

(thunderbird_esr5255+ fixed, thunderbird54 wontfix, thunderbird55 fixed)

RESOLVED FIXED
Thunderbird 55.0
Tracking Status
thunderbird_esr52 55+ fixed
thunderbird54 --- wontfix
thunderbird55 --- fixed

People

(Reporter: bz_bugzilla, Assigned: jorgk-bmo)

References

(Blocks 2 open bugs)

Details

Attachments

(8 files, 2 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36 Steps to reproduce: Using Thunderbird 52.1.1 (32-bit) on Win 7 Ent (64 bit) platform. Yesterday I received a spam e-mail with a couple of girly pictures attached. I clicked on the message accidentally (I usually do a ctrl-U on spam) Actual results: and was surprised to see the pictures displayed despite the options that should have prevented the decoding of the HTML content and the display of the pictures. Expected results: Blank message body should have been displayed with no text and no pictures. It is probable that a malware attachment could just as easily have been allowed to do its thing! The options should be honored.
Component: Untriaged → Security
Summary: After latest update, Thunderbird, in text only mode, with Display Attachments in Line NOT checked, Options privacy Allow remote content in message NOT checked, displayed pictures in an e-mail I received. → displayed pictures in an e-mail, despite Options privacy Allow remote content in message NOT checked. (text only mode, Display Attachments in Line NOT checked)
Group: mail-core-security
Interesting bug. Did you look at the MIME structure in the e-mail you attached? The e-mail has one part that says it is text/plain when in fact it is HTML: Content-Transfer-Encoding: base64 Content-Type: text/plain; charset="utf-8" The image is contained in the message and embedded into the HTML, so it will be displayed. We have bug 322533 on file for blocking inline images. What I don't understand here is why the plaintext part is shown as HTML. Alfred, care to take a look? Even if I edit the content part to text/html, selecting plaintext view doesn't reduce the HTML to plaintext which it usually does.
Flags: needinfo?(infofrommozilla)
Summary: displayed pictures in an e-mail, despite Options privacy Allow remote content in message NOT checked. (text only mode, Display Attachments in Line NOT checked) → HTML contained in text/plain part is rendered as HTML when view as plain text is selected
Status: UNCONFIRMED → NEW
Ever confirmed: true
TB 45 doesn't behave any better, so this is not a regression.
(In reply to Jorg K (GMT+2) from comment #3) > TB 45 doesn't behave any better, so this is not a regression. I would think that with the options I have set, nothing in the text of the message should 'fool' TB into displaying the picture! If it can do that, in this case, even worse if it has been around for a while.....
No HTML is interpreted here. The HTML code is not working at all. The example is an 'multipart/alternative;' mail. The first part is another 'm/a;' capsule. The second part is the picture: | Content-Type: image/jpeg; name="BE9DF8FF2F9AACF85BF91BCF08470FE2.jpg" | Content-Disposition: inline; [...] This part TB shows.
Flags: needinfo?(infofrommozilla)
Here I removed the inner 'm/a;' capsule. Now you can switch between the plain/text and the Picture. I would say that the original does not work, because the first part is invalid. Therefore, the second part is always displayed: the picture. Furthermore, one can see that the HTML text can not be interpreted because of the BASE64 garbage.
Attachment #8870922 - Attachment mime type: message/rfc822 → text/plain
(In reply to Jorg K (GMT+2) from comment #2) > We have bug 322533 on file for blocking inline images. Indeed, because of: | Content-Disposition: inline; [...] this is a Dupe of Bug 322533.
Attached file Corrected version of e-mail (deleted) —
Alfred, thanks for looking into it. I didn't spot the multipart/alternative. I've now corrected the MIME structure of the e-mail: - Changed multipart/alternative to multipart/related - changed text/plain to text/html - changed base64 to quoted-printable. With this correct MIME structure, TB works correctly.
Well, the original message attached here shows two problems: Firstly TB will always show embedded images if the image is also sent as part of the message. That's bug 322533. The second problem is that the message presented here has an incorrect MIME structure, I pointed out the three corrections necessary to make the message work correctly. In fact, looking at it more, I could have corrected it even better. If you view the message in TB 45, you can see that the processing has changed, most likely due to bug 253830 where we changed the way message parts are prioritised. But whatever you do, the girl still shows ;-) I know that the reporter is offended by the girl in the bathroom taking a selfie, but we don't have resources to make TB deal better with incorrect MIME structures. So there are two options here: - Dupe this to bug 322533. - Close it as invalid (since the message is invalid). I think we can safely go with the latter.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
Summary: HTML contained in text/plain part is rendered as HTML when view as plain text is selected → Unwanted inline image shown - Make TB more tolerant to incorrect MIME structure: multipart/alternative where last part is an inline image
Attached file Fully corrected version of e-mail (deleted) —
This is the fully corrected version with multipart/alternative to switch between text and HTML, and the HTML part is a multipart/related including the image. Had the spammer sent it like this, TB would have worked correctly.
Attachment #8870930 - Attachment mime type: message/rfc822 → text/plain
Attachment #8870949 - Attachment mime type: message/rfc822 → text/plain
My concern was that Thunderbird 'ignored' options that supposedly 'protect' unwary users from attachments that may encode and include various forms of malware that can attack and damage the user's computer and spread to other systems. The particular pictures, in this case, are not the point of my taking the time to report the problem. The fact that such a 'bug' can cause major damage is my motivation. If spammers tailor their spam to take advantage of the bug, then I hope that the problem that allowed this to happen is currently under study and will be fixed. In such a case, I have no objection to this report being closed or transferred or what ever is most appropriate in this forum. This being my first venture here, I thank all involved for their time and attention. It's a beautiful day to be alive, isn't it? [Every day!]
Component: Security → MIME
Product: Thunderbird → MailNews Core
Severity: normal → enhancement
Status: RESOLVED → UNCONFIRMED
Ever confirmed: false
Resolution: INVALID → ---
Status: UNCONFIRMED → NEW
Ever confirmed: true
Even though the message may formally be invalid, it looks like spammers do this on purpose in order to exploit Thunderbirds behavior. I too see a potential security risk.
Attached file Simplified test message for debugging (deleted) —
Not sure why my search did not reveal this bug when I opened #1370841, but it sure seems to be the same problem. I attached my (rather simple) test file to facilitate testing and debugging. I still think this is a security problem and should be dealt with the appropriate urgency.
There is no security issue here, not even a privacy issue. The software simply displays what the sender sent. The sender can also send offensive text and that's not censored either. Since this bug exists in prior versions, I checked TB 24, this can hardly be urgent. Other than that, Thunderbird is developed by a group of unpaid volunteers, so we can't make any promises as to what will be fixed and what won't. Also refer to the BMO Etiquette, point 2 "No obligation". If it annoys you, step up and fix it yourself or hire someone to do so.
The point is, if you view mails in plaintext mode, you expect it to never render images, regardless of what the sender sent. There are valid use cases where you do not want images from untrusted senders to be rendered at all, which are mostly security related due to the many RCE bugs in image rendering libraries. Thunderbird simply fails to honor its own setting here. And since that setting is mostly used for security reasons, the bug is a security issue. Just because TB used to dishonor plain text mode for incorrect mime structures, and thus was insecure in the past does in no way change the security impact of this bug. And that a bug existed for a long time and only got revealed now can hardly be used to justify less urgency.
Thanks for the clarification. Now we need to find someone able and willing to fix this, refer to my last paragraph in comment #16.
It was in the hope of finding someone able and willing to fix this problem that we spent the time and energy finding this 'channel' and posting the information about the problem. If this is not the correct 'channel' for posting such problems, or if no one around here has skills enough or cares, then we have wasted our time and energy. I hope this is not the case. Florian: you state the case well. Thunderbird should NEVER render images when the options are set as I had mine set, regardless of what the sender contrives as ways to break the viewer. If the viewer has this vulnerability, then it, almost certainly, has others that will allow malware to be executed. That is a valid security issue. Either the options are misnamed and an additional option needs to be added, named something like "SAFE MODE: NEVER, under any circumstances, do anything other than displaying plain text. Never decode html or other encoding. Never display graphics." or The app needs to be fixed so that it obeys the options as they are currently worded. (none of them currently say "sometimes display graphics if the message format fails to obey certain constraints.") Jorg K: Thank you for pointing out that no one here has an obligation to do anything. I never thought anyone did. I just hope that someone here has the skills and cares, and would want to know about the problem.
Attached patch 1367156-unwanted-images.patch (v1). (obsolete) (deleted) — Splinter Review
Alfred, what do you think of this? Using the test message, plain text view now shows the plain text part, HTML view shows the HTML part. No image is ever displayed. "Show all body parts" will show plain text, HTML and the image as "attachment part". BTW, we do care, but not all bugs are easy to fix and even the smallest fix can cause many regressions especially in treacherous areas like the MIME code.
Attachment #8876309 - Flags: feedback?(infofrommozilla)
Do not get me wrong, I'm not against your wish. I just think the reasoning is a bit excessive. (In reply to Florian from comment #17) > Thunderbird simply fails to honor its own setting here. And since that > setting is mostly used for security reasons, the bug is a security issue. From what setting are you talking about? "Display Attachments Inline" causes the exact opposite. It ensures that attached images which are not part of the mail are displayed anyway. (In reply to bz_tb1 from comment #19) > executed. That is a valid security issue. Viewing an image must not be a security risk. Because the TB used to display the components of the Firefox, he would be exposed to this risk when displaying any HTML page.
Comment on attachment 8876309 [details] [diff] [review] 1367156-unwanted-images.patch (v1). This works for the old examples. But unfortunately not for new (I hope I do not bring the spamer on new ideas). The changes are minimal, so no side effects are expected.
Attachment #8876309 - Flags: feedback?(infofrommozilla) → feedback+
Attachment 8876309 [details] [diff] does not help here unfortunately.
Alfred, thanks for looking into it. (In reply to Alfred Peters from comment #22) > This works for the old examples. But unfortunately not for new (I hope I do > not bring the spamer on new ideas). So you changed multipart/alternative to multipart/x-whatever. Did you notice that when plain text is requested, nothing is displayed for this e-mail, so that's "better" than before. > The changes are minimal, so no side effects are expected. You know as well as I that MIME changes can backfire, see two recent regressions in TB 52 due to MIME changes ;-), bug 1358565, bug 1361422. BTW, I have a try run for this: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=09b17530481d24ca49aff452258d052134c073f2 At time of writing, Linux isn't complete yet, on Mac it's green although it's red, xpcshell-test had an intermittent failure and mozmill is suffering from bug 1371898.
Comment on attachment 8876309 [details] [diff] [review] 1367156-unwanted-images.patch (v1). Aceman, welcome to MIME. The code we're looking at is the code that selects the part to display in a multipart/alternative parent. RFC 1341 https://www.w3.org/Protocols/rfc1341/7_2_Multipart.html states: In general, user agents that compose multipart/alternative entities should place the body parts in increasing order of preference, that is, with the preferred format last. For fancy text, the sending user agent should put the plainest format first and the richest format last. Receiving user agents should pick and display the last format they are capable of displaying. In the case where one of the alternatives is itself of type "multipart" and contains unrecognized sub-parts, the user agent may choose either to show that alternative, an earlier alternative, or both. [snip] It may be the case that some user agents, if they can recognize more than one of the formats, will prefer to offer the user the choice of which format to view. This makes sense, for example, if mail includes both a nicely-formatted image version and an easily-edited text version. What is most critical, however, is that the user not automatically be shown multiple versions of the same data. Either the user should be shown the last recognized version or should explicitly be given the choice. === TB implements the second paragraph: It displays the HTML part of the user selected HTML and the text part if the user selected plain text. However, HTML is not prioritised over rogue parts, so by putting a rogue part last, TB can be tricked into displaying the rogue part. The patch addresses that by prioritising images lower, since typically some sort of (rich) text should be displayed as the message body. It also suppresses images completely when a plain text view is selected. As I said in the previous comment: No test failures due to this. We could enhance test-view-plaintext.js by adding a test e-mail with a rogue part to make sure a non-rogue part is displayed.
Attachment #8876309 - Flags: review?(acelists)
(In reply to Jorg K (GMT+2) from comment #24) > (In reply to Alfred Peters from comment #22) > > But unfortunately not for new > So you changed multipart/alternative to multipart/x-whatever. Did you notice > that when plain text is requested, nothing is displayed for this e-mail, so > that's "better" than before. Yes, indeed. I had probably still activated the HTML view. Sorry for the noise.
I assume we do not want to completely reject messages with invalid MIME structure as surely some important sending client produces/produced invalid messages from time to time. And we want to display as many messages as possible if we can guess what the invalid MIME structure contains.
Comment on attachment 8876309 [details] [diff] [review] 1367156-unwanted-images.patch (v1). Review of attachment 8876309 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, this works nicely. Can you add the case into the test file? Using some neutral image of course ;)
Attachment #8876309 - Flags: review?(acelists) → review+
I believe that there are at least two long standing security vulnerabilities associated with display of html and display of emailed images. One is that accessing any embedded URL can alert the spammers that they have 'found a live one' and makes your e-mail address a marketable commodity. Spammers often use 'unique tagged URLs' to enable them to know that their spam has been viewed and by whom it was viewed. That is one reason that I NEVER allow TB to run in the mode allowing html decoding. The other is that there are known vulnerabilities in software that displays graphic images. Those vulnerabilities have been used to convey and execute malware. Those are a couple of the reasons that TB has long had options allowing the user to disable the decoding of html and enabling the user to disable the display of images. The unexpected appearance of the image was therefore a 'red flag' saying "TB no longer works as it was designed to work! Someone needs to know about this!!"
Attached patch 1367156-unwanted-images.patch (v1) + test (obsolete) (deleted) — Splinter Review
Here's your test.
Assignee: nobody → jorgk
Attachment #8876309 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8876422 - Flags: review?(acelists)
Sorry about the noise, I've added another simpler test.
Attachment #8876422 - Attachment is obsolete: true
Attachment #8876422 - Flags: review?(acelists)
Attachment #8876429 - Flags: review?(acelists)
Comment on attachment 8876429 [details] [diff] [review] 1367156-unwanted-images.patch (v1) + 2 tests Review of attachment 8876429 [details] [diff] [review]: ----------------------------------------------------------------- Perfect, thanks.
Attachment #8876429 - Flags: review?(acelists) → review+
Status: ASSIGNED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 55.0
Comment on attachment 8876429 [details] [diff] [review] 1367156-unwanted-images.patch (v1) + 2 tests By popular demand I'll consider this for ESR uplift in due course. MIME changes are always risky, so let this be tested on a few beta versions first starting with TB 55 beta due out next week. No need to uplift to TB 54 beta.
Attachment #8876429 - Flags: approval-comm-esr52?
Attachment #8876429 - Flags: approval-comm-esr52? → approval-comm-esr52+
Please don't publish full messages in comments, attach them instead. And you've done it twice making the bug hard to read. Please don't change the bug status, if you don't understand how release management works. As the tracking flags show, this should be fixed in TB 55 beta and TB 56 Daily. Not fixed yet in TB 52.x since the fix hasn't been released in that version. I tested with the message in comment #35 and see no image using TB 56 Daily.
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Restrict Comments: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: