HTML code injection by renaming attached file

RESOLVED FIXED in Thunderbird 26.0

Status

Thunderbird
Message Compose Window
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: abillings, Assigned: standard8)

Tracking

({sec-low})

unspecified
Thunderbird 26.0
sec-low

Thunderbird Tracking Flags

(thunderbird24 fixed, thunderbird25 fixed, thunderbird-esr1724+ fixed)

Details

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

4 years ago
Created attachment 784428 [details]
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

4 years ago
Why Core: Editor? isn't this Thunderbird bug?

Comment 2

4 years ago
What mean CORE for this Bug?
(Reporter)

Comment 3

4 years ago
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.
Component: Editor → Message Compose Window
Product: Core → Thunderbird

Comment 4

4 years ago
I also believe that there  no is a  problem in the editor.
(Assignee)

Comment 5

4 years ago
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
(Assignee)

Comment 6

4 years ago
Created attachment 785698 [details]
Eml Test Case

Simple test case for manual reproduction.
(Assignee)

Comment 7

4 years ago
Created attachment 785699 [details] [diff] [review]
Escape HTML in the attachment names

This ensures that we escape any html in the attachment names before they get injected into the HTML.
Assignee: nobody → mbanner
Status: NEW → ASSIGNED
Attachment #785699 - Flags: review?(neil)
(Assignee)

Comment 8

4 years ago
Created attachment 785701 [details] [diff] [review]
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.
Attachment #785701 - Flags: review?
(Assignee)

Updated

4 years ago
status-thunderbird24: --- → affected
status-thunderbird25: --- → affected
status-thunderbird-esr17: --- → affected
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Updated

4 years ago
Attachment #785701 - Flags: review? → review?(Pidgeot18)

Comment 9

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

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

Comment 10

4 years ago
Created attachment 785708 [details] [diff] [review]
Escape HTML in the attachment names v2

Yes I did, bad c&p.
Attachment #785699 - Attachment is obsolete: true
Attachment #785708 - Flags: review?(neil)
Attachment #785699 - Flags: review?(neil)
(Assignee)

Comment 11

4 years ago
Created attachment 785714 [details] [diff] [review]
Escape HTML in the attachment names v2

This time with the change included.
Attachment #785708 - Attachment is obsolete: true
Attachment #785708 - Flags: review?(neil)
Attachment #785714 - Flags: review?(neil)

Updated

4 years ago
Attachment #785714 - Flags: review?(neil) → review+

Comment 12

4 years ago
What sec-??? is considered?
(Assignee)

Comment 13

4 years ago
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.
Flags: needinfo?(dveditz)

Comment 14

4 years ago
As you may consider it an HTML injection to sec-low?
(Assignee)

Comment 15

4 years ago
(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 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.
Attachment #785701 - Flags: review?(Pidgeot18) → review+

Comment 17

4 years ago
This issue is considered for a Bounty?
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.
Group: core-security
Keywords: sec-low
Flags: needinfo?(dveditz)

Comment 19

4 years ago
(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?
(Assignee)

Comment 20

4 years ago
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

4 years ago
But you do not talk about sec-low (only sec-critical/high) too in wiki.mozilla.org
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

4 years ago
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

4 years ago
(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."
(Assignee)

Comment 25

4 years ago
https://hg.mozilla.org/comm-central/rev/b97f9f43d5e1
https://hg.mozilla.org/comm-central/rev/938c436da6b1
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 26.0
(Assignee)

Comment 26

4 years ago
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.
Attachment #785714 - Flags: approval-comm-beta+
Attachment #785714 - Flags: approval-comm-aurora+
(Assignee)

Updated

4 years ago
Attachment #785701 - Flags: approval-comm-beta+
Attachment #785701 - Flags: approval-comm-aurora+
(Assignee)

Comment 27

4 years ago
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
status-thunderbird24: affected → fixed
status-thunderbird25: affected → fixed
tracking-thunderbird-esr17: --- → 24+
(Assignee)

Comment 28

4 years ago
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.
Attachment #785701 - Flags: approval-comm-esr17+
(Assignee)

Updated

4 years ago
Attachment #785714 - Flags: approval-comm-esr17+
(Assignee)

Comment 29

4 years ago
https://hg.mozilla.org/releases/comm-esr17/rev/6294376dd781
https://hg.mozilla.org/releases/comm-esr17/rev/22c6a1b599ef
status-thunderbird-esr17: affected → fixed
You need to log in before you can comment on or make changes to this bug.