900552 - HTML code injection by renaming attached file

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Al Billings [:abillings - ex-MoCo]]

Attachment: PoC video

Security@ received the following email from the researcher, cc'd on the bug:

I have found a HTML Code Injection, to attach a file in a new mail, to rename the attached file this implies that you can insert HTML without being filtered properly. This vulnerability allows successful phishing attack.


Steps to reproduce:

 1) Compose a new mail
 2) Attach
 3) Select a file ... (.exe, .png, .html, etc, etc.)
 4) Rename the attached file  with HTML Code (right click, rename)

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Why Core: Editor? isn't this Thunderbird bug?

Comment 2

[Fabian Cuchietti]

What mean CORE for this Bug?

Comment 3

[Al Billings [:abillings - ex-MoCo]]

Olli, I'm fuzzy where the lines in some of this code is. If you think this is specific to Thunderbird and that Seamonkey, for example, is not vulnerable to this, please change the product and component.

Fabian, this is just where the code for the problem lives. It makes it easier to get this assigned to the right person to fix.

Comment 4

[Fabian Cuchietti]

I also believe that there  no is a  problem in the editor.

Comment 5

[Mark Banner (:standard8)]

I've just tried testing this. This does only affects

- Users with "Display attachments inline" turned on

and one of:

- Users who have allowed remote content to always be loaded from the sender
- Users who choose to load the remote content for the message

Comment 6

[Mark Banner (:standard8)]

Attachment: Eml Test Case

Simple test case for manual reproduction.

Comment 7

[Mark Banner (:standard8)]

Attachment: Escape HTML in the attachment names

This ensures that we escape any html in the attachment names before they get injected into the HTML.

Comment 8

[Mark Banner (:standard8)]

Attachment: Automated test case

Joshua, this seemed to be the easiest place to test for the replacement, but I wondered if you knew of any other ways so that we could check the other cases.

Comment 9

[[e-mail]]

Comment on attachment 785699 [details] [diff] [review]
Escape HTML in the attachment names

>+      UtilityWrite(escapedName);
>       mHTMLHeaders.Append(name);
Did you mean mHTMLHeaders.Append(escapedName); ?

Comment 10

[Mark Banner (:standard8)]

Attachment: Escape HTML in the attachment names v2

Yes I did, bad c&p.

Comment 11

[Mark Banner (:standard8)]

Attachment: Escape HTML in the attachment names v2

This time with the change included.

Comment 12

[Fabian Cuchietti]

What sec-??? is considered?

Comment 13

[Mark Banner (:standard8)]

I think this is probably sec-low or maybe sec-moderate, but I'd value input from Dan here as he's got more experience than me with these.

Comment 14

[Fabian Cuchietti]

As you may consider it an HTML injection to sec-low?

Comment 15

[Mark Banner (:standard8)]

(In reply to Fabian Cuchietti from comment #14)
> As you may consider it an HTML injection to sec-low?

Sorry, yes, I should have explained that. It is an html injection, but javascript is not available (I've just confirmed that). So from a user perspective, they attacker could spoof some html, but it'd appear in the message reader section and they could just do that via writing an html message.

Although the attacker could cause remote content to be loaded, a) we ask the user by default and b) javascript isn't available even within the remote content.

Therefore, this seems to fit the "Identification of users by profiling browsing behavior." category of sec-low.

Comment 16

[Joshua Cranmer [:jcranmer]]

Comment on attachment 785701 [details] [diff] [review]
Automated test case

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

The way this is implemented may be surprising to some API consumers, but I don't think it will matter that much in practice.

Comment 17

[Fabian Cuchietti]

This issue is considered for a Bounty?

Comment 18

[Matt Wobensmith [:mwobensmith][:matt:]]

As per Dan, sec-low, unintended but valid bug, minimal effect because we accept HTML content in the mail body and therefore filter dangerous content.

Comment 19

[Fabian Cuchietti]

(In reply to Matt Wobensmith from comment #18)
> As per Dan, sec-low, unintended but valid bug, minimal effect because we
> accept HTML content in the mail body and therefore filter dangerous content.

I don't understand. Is valid for a bounty?

Comment 20

[Mark Banner (:standard8)]

The policy is available here which will give you the general guidelines for when bounties are awarded:

http://www.mozilla.org/security/bug-bounty.html

Comment 21

[Fabian Cuchietti]

But you do not talk about sec-low (only sec-critical/high) too in wiki.mozilla.org

Comment 22

[Matt Wobensmith [:mwobensmith][:matt:]]

Fabian, only bugs that have been rated high/critical will receive a bounty. Therefore, to answer your question, this bug will not receive one.

Comment 23

[Joe Sabash [:JoeS1]]

I was a little concerned that this might interfere with native ogg/audio or the use of iframe in the html body.
Those functions work normally, and the code injection in the attachment header fix seems to work as designed.
tested with  http://hg.mozilla.org/mozilla-central/rev/45128af17739
with this patch imported to a local build.

Comment 24

[Fabian Cuchietti]

(In reply to Matt Wobensmith from comment #22)
> Fabian, only bugs that have been rated high/critical will receive a bounty.
> Therefore, to answer your question, this bug will not receive one.

My previous bounty was to "sec-moderate."

Comment 25

[Mark Banner (:standard8)]

https://hg.mozilla.org/comm-central/rev/b97f9f43d5e1
https://hg.mozilla.org/comm-central/rev/938c436da6b1

Comment 26

[Mark Banner (:standard8)]

Comment on attachment 785714 [details] [diff] [review]
Escape HTML in the attachment names v2

[Triage Comment]
Taking onto branches to get some testing before next releases.

Comment 27

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-aurora/rev/b4988a793e38
https://hg.mozilla.org/releases/comm-aurora/rev/31638cf345d7
https://hg.mozilla.org/releases/comm-beta/rev/3ec36521e795
https://hg.mozilla.org/releases/comm-beta/rev/7f9f2db26a67

Comment 28

[Mark Banner (:standard8)]

Comment on attachment 785701 [details] [diff] [review]
Automated test case

[Triage Comment]
No complaints afaik, so we'll take this to esr 17 as well.

Comment 29

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-esr17/rev/6294376dd781
https://hg.mozilla.org/releases/comm-esr17/rev/22c6a1b599ef