303752 - Forward inline attaches externally referenced local files and intranet files

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[Jesse Ruderman]

Thunderbird version 1.0+ (20050804)

Steps to reproduce:

1. Telnet to an SMTP server and send yourself a simple HTML message containing

foo <img src="file:///..." alt="bar" width="1" height="1"> baz

2. Using Thunderbird, retrieve the message.
3. Select Forward (inline) from the menu, or just Forward if you set your pref
to forward inline by default.
4. Send the forward as HTML.
5. View the source of the resulting message.

Result: It includes the contents of the local file that was referenced.

Scope:
* I tested with GIF and HTML files, so I imagine this hole allows an attacker to
steal a file of any type as long as he knows its URL.
* I tested with both http://localhost/* URLs and file:///* URLs.  Both work, so
this hole allows an attacker to steal intranet pages as well as local files.

I haven't thought too much about how hard it would be for an attacker to
convince someone to forward such a message to an address controlled by the
attacker.  I think it wouldn't be hard to come up with a scheme that worked at
least 10% of the time, though.

Comment 1

[Jesse Ruderman]

This bug is similar to bug 88124.

Comment 2

[Jesse Ruderman]

Attachment: Notes about how I tested this bug

Comment 3

[Gervase Markham [:gerv]]

One of those "forward this to ten friends or you will have bad luck" emails
should get a pretty high success rate for this exploit. Combine it with a "and
forward it to this address so we can see where it's got to" text and you'll have
a good collection of people's files.

Gerv

Comment 4

[Mike Shaver (:shaver emeritus)]

David: this is something we should really fix for beta, can you take a look at
this ASAP?

Comment 5

[Scott MacGregor]

Attachment: possible fix

This patch extends Jean-Francois fix for this same problem (but for replies) in
Bug 88124 many years ago to also work on forwarded HTML messages. In my test,
the image still loads in the compose window from my hard drive but when I
actually send the forwarded message, we walk through the embedded objects, see
that this was not something that came from the user so we fail to attach it to
the outgoing message. 

This is consistent with the behavior seen when replying intead of forwarding.

Comment 6

[Scott MacGregor]

Comment on attachment 192420 [details] [diff] [review]
possible fix

David, I'd like to experiment with this potential fix on the trunk. What do you
think?

Comment 7

[David :Bienvenu]

Comment on attachment 192420 [details] [diff] [review]
possible fix

seems worth a try

Comment 8

[Scott MacGregor]

I've put this patch on the 1.8 branch too

Comment 9

[Scott MacGregor]

I forgot to mark this closed since we checked in a fix. Although we may end up
re-opening it if some of these regression bugs look serious enough.

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 192420 [details] [diff] [review]
possible fix

a=dveditz for drivers for aviary101 and moz17 branches

Comment 11

[David :Bienvenu]

this fix looks like it's on the 1.7 branch and the 1.0x branch already. Am I missing something?

Comment 12

[David :Bienvenu]

there were two calls to TagEmbeddedObjects on the trunk and one on the branches. I put the code as checked in (which is slightly different from this patch) onto the 1.08 and 1.7 branches.

Comment 13

[David :Bienvenu]

the difference from this patch and what I checked in was to include the fix for the regression introduced by this change, i.e., bug 305557 - so that regression shouldn't happen on the branches. 

Comment 14

[Scott MacGregor]

David, when you landed this on 1.0.8 and 1.7.13, did you check in Bug 315625 as well as 305557? I think that was another regression that was caused by 305557. I'll try to look again tomorrow.

Comment 15

[David :Bienvenu]

I didn't check in Bug 315625 but it looks like you did on 1/31 :-) the 1.5.0.1 branch contains the fix.

Comment 16

[Scott MacGregor]

I meant 1.0.8 not 1.5.0.x. I'm pretty sure your the one that landed it on the 1.0.x / 1.7.13 branches :)

Comment 17

[David :Bienvenu]

oy, too many branches. I definitely did not land in 1.08

Comment 18

[Marcia Knous [:marcia]]

David: I am confused - I took a quick look in bonsai for 1.0.8 and don't see this checkin.  Can you clarify?

Comment 19

[David :Bienvenu]

I did NOT land on the 1.08 branch - removing keyword. Is it too late to land in 1.08? Or did you land it, Scott?

Comment 20

[Scott MacGregor]

David, why do you think this isn't fixed on the 1.0.x. branch?

It sure looks like it was:

http://lxr.mozilla.org/aviary101branch/source/mailnews/compose/src/nsMsgCompose.cpp#539

comment 11 comment 12 and comment 13 also support the fact that this was checked in. And because I was seeing the regression this bug caused on the 1.0.8 branch, so  landed 315625 to fix that regression. That gives me even further confidence that this is in 1.0.8 :)

Comment 21

[Scott MacGregor]

my happy mouse clicks accidentally marked this bug as verified. re-opening, re-closing to get back to the fixed state without verification.

Comment 22

[David :Bienvenu]

duh, sorry, of course it is fixed in 1.08 - it's the fix for Bug 315625 that's not on the 1.08 branch.

Comment 23

[David :Bienvenu]

or rather, I didn't check the fix for  Bug 315625  on the 1.08 branch - you did :-)

Comment 24

[Marcia Knous [:marcia]]

Sorry to raise the red flag on this one, I was just trying to understand/make sure the appropriate checkins made it in. Sorry for the noise.

Comment 25

[Tracy Walker [:tracy]]

Verified with Windows Thunderbird - version 1.0.8 (20060317)