Last Comment Bug 452093 - (CVE-2010-0181) Redirect to mailto:address in linked image opens email editor
(CVE-2010-0181)
: Redirect to mailto:address in linked image opens email editor
Status: RESOLVED FIXED
[sg:low]
: verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.9.3a2
Assigned To: Brandon Sterne (:bsterne)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-08-25 09:26 PDT by Henry Sudhof
Modified: 2010-07-15 13:54 PDT (History)
15 users (show)
dveditz: wanted1.9.0.x+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.2-fixed
.9-fixed


Attachments
HTML testcase using tinyurl (247 bytes, text/html)
2008-08-25 09:28 PDT, Henry Sudhof
no flags Details
Using mailto link in an image (74 bytes, text/html)
2008-09-29 12:06 PDT, cmtalbert
no flags Details
fix (1.18 KB, patch)
2010-02-09 10:42 PST, Brandon Sterne (:bsterne)
cbiesinger: review-
Details | Diff | Splinter Review
fix v2 (1.21 KB, patch)
2010-02-09 11:38 PST, Brandon Sterne (:bsterne)
bzbarsky: review+
Details | Diff | Splinter Review
fix v3 (1.20 KB, patch)
2010-02-09 12:34 PST, Brandon Sterne (:bsterne)
brandon: review+
Details | Diff | Splinter Review
fix v4 (1.17 KB, patch)
2010-02-10 09:36 PST, Brandon Sterne (:bsterne)
brandon: review+
joe: review+
mbeltzner: approval1.9.2.2+
mbeltzner: approval1.9.1.9+
mbeltzner: approval1.9.0.19-
Details | Diff | Splinter Review

Description Henry Sudhof 2008-08-25 09:26:56 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1

I was notified by an user ("Reelix") about a weird behavior with img tags. I wasn't able to find a duplicate, so I thought it might be better to file a report here, despite you probably being aware of the issue.

If the request for the image is answered with a response redirecting to a mailto: address, then the default email editor opens.

i.e.
<img src="<page that redirects to mailto:foo@bar>" /> will cause firefox to open the default email application.

or more verbatim:
a t.html
--snip
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
   "http://www.w3.org/TR/html4/loose.dtd">
<html>
   <head>
      <title>mail test</title>
   </head>
   <body>
  <img src="t.php" />
   </body>
</html>
--snap

Where t.php redirects to mailto:foo@bar.com?subject=Irritating%20popup&body=I%20d
o%20not%20think%20image%20tags%20should%20be%20able%20to%20t
his .

Opens the email editor. I don't think that image tags should be able to open any external applications and/or cause popups.

Thanks for looking into it,
cheers
~H



Reproducible: Always

Steps to Reproduce:
1. write a page with an image tag like <img src="foo.php" />
2. have foo.php redirect to mailto:foo@bar.com (header('location:mailto:foo@bar.com');)
3. View the page 
Actual Results:  
The external email editor opens.

Expected Results:  
Broken image.
Comment 1 Henry Sudhof 2008-08-25 09:28:18 PDT
Created attachment 335370 [details]
HTML testcase using tinyurl
Comment 2 cmtalbert 2008-09-03 16:34:52 PDT
Confirming.  

This definitely looks like something that should be forbidden.  Cc'ing dveditz for his professional opinion.
Comment 3 cmtalbert 2008-09-03 16:37:39 PDT
Confirmed on windows vista and mac:
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b1pre) Gecko/20080903034741 Minefield/3.1b1pre

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9.1a2pre) Gecko/20080819152019 Minefield/3.1a2pre

sorry for bugspam
Comment 4 Henry Sudhof 2008-09-26 04:14:51 PDT
I am very sorry about being a pain, but is there any movement on this?

Cheers,
~H
Comment 5 cmtalbert 2008-09-29 12:06:08 PDT
Created attachment 340966 [details]
Using mailto link in an image

This is a test with a image that has a mailto: link as the source, you can see that there is a broken link in this case.  So, the question is do we want to detect the redirect and show a broken image in this case or not?
Comment 6 Brandon Sterne (:bsterne) 2009-01-08 11:51:27 PST
Doesn't look like there's been much movement here (though dveditz didn't get cc'd per comment 2... he's cc'd now).

I'm going to set this as sg:low for now, since this is more of a DoS/annoyance than anything.
Comment 7 Henry Sudhof 2009-02-19 10:46:48 PST
I have to say: this is potentially a very bad annoyance. Spammers could use this easily to bypass the popup blocker. Current measures like noscript wouldn't help a bit.
Comment 8 Henry Sudhof 2009-07-21 02:25:28 PDT
Sorry for being a pest, but we ("phpBB") keep getting reports about this being used for spam/popups with an increasing frequency.

Cheers,
~H
Comment 9 Henry Sudhof 2010-02-08 10:22:48 PST
Hi,
I'm sorry, but I have  to give this its semi-annual bump. It is by now commonly used in spam entries on forums. 

I am by now tempted to contact the media about the issue. I'm sure that there are many pressing problems, but this renders the popup blocker useless and is developing into a real problem.

Cheers,
all the best,
~H
Comment 10 Will Hough 2010-02-08 10:31:53 PST
Confirmed on Ubuntu 9.10 with Firefox 3.5.7 opening Empathy mail client.
Comment 11 Will Hough 2010-02-08 10:47:57 PST
[edit]

The Evolution mail client, not Empathy.
Comment 12 Brandon Sterne (:bsterne) 2010-02-08 14:36:42 PST
I'm going to take a stab at this.
Comment 13 Brandon Sterne (:bsterne) 2010-02-09 10:42:29 PST
Created attachment 426017 [details] [diff] [review]
fix
Comment 14 Christian :Biesinger (don't email me, ping me on IRC) 2010-02-09 10:53:04 PST
Comment on attachment 426017 [details] [diff] [review]
fix

Don't hardcode protocol names. Instead, I think you want to check for the URI_DOES_NOT_RETURN_DATA protocol handler flag. (Something like http://mxr.mozilla.org/mozilla-central/source/parser/htmlparser/src/nsViewSourceHTML.cpp#1074)
Comment 15 Christian :Biesinger (don't email me, ping me on IRC) 2010-02-09 10:53:59 PST
Or maybe URI_NON_PERSISTABLE is better.
Comment 16 Boris Zbarsky [:bz] 2010-02-09 11:04:43 PST
In fact, this should be using the same code as nsNoDataProtocolContentPolicy::ShouldLoad.

Heck, ideally it should be _calling_ that ShouldLoad....  As well as perhaps other content policy methods.  Sadly, content policy doesn't really mesh that well with redirects.
Comment 17 Brandon Sterne (:bsterne) 2010-02-09 11:22:23 PST
My first patch copied code from nsNoDataProtocolContentPolicy::ShouldLoad, in fact.  I agree that the ideal fix would be to call that function, but we lack the context we need to do so during the redirect.

Shall we go forward, then, with the patch suggested in comment 14?
Comment 18 Boris Zbarsky [:bz] 2010-02-09 11:26:11 PST
> My first patch copied code from nsNoDataProtocolContentPolicy::ShouldLoad

No, it copied _part_ of that code.  The fast-path.  You either need to copy all the code or just copy the non-fast-path part (depending on how performance-critical we think image redirects are).

I'm not entirely convinced that the fast-path is even important in ShouldLoad there, btw.
Comment 19 Brandon Sterne (:bsterne) 2010-02-09 11:38:49 PST
Created attachment 426038 [details] [diff] [review]
fix v2
Comment 20 Boris Zbarsky [:bz] 2010-02-09 11:45:22 PST
So questions:

1) Why the original URI and not the URI?
2) Does this not leave the issue for stylesheet loads, script loads, etc?
Comment 21 Brandon Sterne (:bsterne) 2010-02-09 12:03:47 PST
(In reply to comment #20)
> 1) Why the original URI and not the URI?
I don't have a reason why other originalURI was being used in another check in this function already.  It makes more sense to use URI since that will take into account URI resolution. 

> 2) Does this not leave the issue for stylesheet loads, script loads, etc?
It does leave this problem for those other types of loads.  I think the reason this bug was filed with images in mind is that this is a particular problem for forums and such who allow users to post images, but not necessarily those other types of content.
Comment 22 Boris Zbarsky [:bz] 2010-02-09 12:06:46 PST
Ah, I see.  OK, then this is ok for now but we should get a followup bug filed to tag loads that shouldn't permit no-data stuff somehow and just have the global redirect observer enforce this restriction.... or something.
Comment 23 Boris Zbarsky [:bz] 2010-02-09 12:07:05 PST
Comment on attachment 426038 [details] [diff] [review]
fix v2

r=me if you use the URI instead of the originalURI.
Comment 24 Brandon Sterne (:bsterne) 2010-02-09 12:34:48 PST
Created attachment 426047 [details] [diff] [review]
fix v3

Addresses bz's comments.
Comment 25 Brandon Sterne (:bsterne) 2010-02-09 12:43:09 PST
Boris, re: comment 22, there is bug 456957, which is to make Content Policy work with redirects.  Is that sufficient, or shall I open up a follow-up bug to tag loads (channels?) with dont-allow-nodata and have a redirect observer set up to enforce those?
Comment 26 Christian :Biesinger (don't email me, ping me on IRC) 2010-02-09 14:54:41 PST
should imagelib really use a DOM error code?
Comment 27 Brandon Sterne (:bsterne) 2010-02-09 16:15:44 PST
I have no attachment to that particular error code.  I'm open to other suggestions.
Comment 28 Christian :Biesinger (don't email me, ping me on IRC) 2010-02-10 04:09:05 PST
Comment on attachment 426047 [details] [diff] [review]
fix v3

I'd probably just go with NS_ERROR_ABORT, or if you prefer a new error code added to ImageErrors.h.
Comment 29 Brandon Sterne (:bsterne) 2010-02-10 09:36:13 PST
Created attachment 426262 [details] [diff] [review]
fix v4

Changed error code.  Carrying forward Boris' r+.
Comment 30 Reed Loden [:reed] (use needinfo?) 2010-02-11 14:00:43 PST
http://hg.mozilla.org/mozilla-central/rev/c492fb6295d1
Comment 31 Boris Zbarsky [:bz] 2010-02-11 18:32:18 PST
> there is bug 456957, which is to make Content Policy work with redirects.  Is
> that sufficient,

I'd prefer a separate bug with narrower scope to handle just the no-data thing, I think.
Comment 32 Brandon Sterne (:bsterne) 2010-02-12 09:11:33 PST
Filed follow-up bug 545871.
Comment 33 Mike Beltzner [:beltzner, not reading bugmail] 2010-02-26 13:32:48 PST
Comment on attachment 426262 [details] [diff] [review]
fix v4

a=beltzner for all branches, why aren't there tests?

I bet you'll add some tests. :)
Comment 35 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-10 12:55:07 PST
Comment on attachment 426262 [details] [diff] [review]
fix v4

This missed the deadline.
Comment 36 Al Billings [:abillings] 2010-03-15 17:25:16 PDT
Verified for 1.9.2 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.2pre) Gecko/20100315 Namoroka/3.6.2pre (.NET CLR 3.5.30729) using attached testcase.

Verified for 1.9.1 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.9pre) Gecko/20100311 Shiretoko/3.5.9pre (.NET CLR 3.5.30729).
Comment 37 chris hofmann 2010-05-18 12:27:25 PDT
mustlive posted on bugtraq that other browsers are affected http://www.securityfocus.com/archive/1/511327
Comment 38 Henry Sudhof 2010-05-18 13:33:24 PDT
I would argue that the report by mustlive is less critical/ of a vulnerability. Posting images is a common feature for "normal" users on blogs, forums etc. The img src vector allowed circumventing the popup blocker and DoS on such sites.
 
By contrast, being able to post an iframe with an arbitrary source is enough for conducting far worse attacks on the site and its visitors - including DoS, but extending to attacks on common plugins, XSS and worse.



Cheers,
~H

Note You need to log in before you can comment on or make changes to this bug.