Closed Bug 452093 (CVE-2010-0181) Opened 16 years ago Closed 14 years ago

Redirect to mailto:address in linked image opens email editor

Categories

(Core :: Graphics: ImageLib, defect)

Product:
Core
Component:
Graphics: ImageLib
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.9.3a2
Tracking Flags:
Tracking Status
status1.9.2 --- .2-fixed
status1.9.1 --- .9-fixed

People

(Reporter: kellanved, Assigned: bsterne)

Details

(Keywords: verified1.9.1, verified1.9.2, Whiteboard: [sg:low])

Attachments

(3 files, 3 obsolete files)

HTML testcase using tinyurl
247 bytes, text/html
Details
Using mailto link in an image
74 bytes, text/html
Details
fix v4
1.17 KB, patch
bsterne
: review+
joe
: review+
beltzner
: approval1.9.2.2+
beltzner
: approval1.9.1.9+
beltzner
: approval1.9.0.19-
Details | Diff | Splinter Review 
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.

    
  
Version: unspecified → 3.0 Branch

    
    

    
  
Confirming.  

This definitely looks like something that should be forbidden.  Cc'ing dveditz for his professional opinion.
Status: UNCONFIRMED → NEW
Component: General → File Handling
Ever confirmed: true
Product: Firefox → Core
QA Contact: general → file-handling
Version: 3.0 Branch → Trunk

    
    

    
  
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

    
    

    
  
I am very sorry about being a pain, but is there any movement on this?

Cheers,
~H

    
    

    
  


  
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?

    
    

    
  
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.
Whiteboard: [sg:low]

    
    

    
  
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.

    
    

    
  
Sorry for being a pest, but we ("phpBB") keep getting reports about this being used for spam/popups with an increasing frequency.

Cheers,
~H

    
    

    
  
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

    
    

    
  
Confirmed on Ubuntu 9.10 with Firefox 3.5.7 opening Empathy mail client.

    
    

    
  
[edit]

The Evolution mail client, not Empathy.

    
    

    
  
I'm going to take a stab at this.
Assignee: nobody → bsterne

    
    

    
  

      
      
      
      

        Attached patch
          
          
        fix (obsolete)
        — 
          — Splinter Review
      

    


  

        Attachment #426017 -
        Flags: review?(jst)

    
    

    
  
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)

        Attachment #426017 -
        Flags: review?(jst) → review-

    
    

    
  
Or maybe URI_NON_PERSISTABLE is better.

    
    

    
  
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.

    
    

    
  
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?

    
    

    
  
> 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.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        fix v2 (obsolete)
        — 
          — Splinter Review
      

    


  

        Attachment #426017 -
        Attachment is obsolete: true

        Attachment #426038 -
        Flags: review?(bzbarsky)

    
    

    
  
So questions:

1) Why the original URI and not the URI?
2) Does this not leave the issue for stylesheet loads, script loads, etc?

    
    

    
  
(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.

    
    

    
  
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 on attachment 426038 [details] [diff] [review]
fix v2

r=me if you use the URI instead of the originalURI.

        Attachment #426038 -
        Flags: review?(bzbarsky) → review+

    
    

    
  

      
      
      
      

        Attached patch
          
          
        fix v3 (obsolete)
        — 
          — Splinter Review
      

    


  
Addresses bz's comments.

        Attachment #426038 -
        Attachment is obsolete: true

        Attachment #426047 -
        Flags: review+

    
    

    
  
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?

    
    

    
  
should imagelib really use a DOM error code?

    
    

    
  
I have no attachment to that particular error code.  I'm open to other suggestions.

    
    

    
  
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.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        fix v4
        — 
          — Splinter Review
      

    


  
Changed error code.  Carrying forward Boris' r+.

        Attachment #426047 -
        Attachment is obsolete: true

        Attachment #426262 -
        Flags: review+

    
  

          status1.9.1:
          --- → ?

          status1.9.2:
          --- → ?

          status2.0:
          --- → ?
Keywords: checkin-needed

    
  

          status1.9.1:
          ? → ---

          status1.9.2:
          ? → ---

          status2.0:
          ? → ---

    
  

        Attachment #426262 -
        Flags: review?(joe)

        Attachment #426262 -
        Flags: review?(joe) → review+

    
    

    
  
http://hg.mozilla.org/mozilla-central/rev/c492fb6295d1
Status: NEW → RESOLVED
Closed: 14 years ago
Component: File Handling → ImageLib
Keywords: checkin-needed
OS: Windows Vista → All
QA Contact: file-handling → imagelib
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a2

    
    

    
  
> 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.

    
    

    
  
Filed follow-up bug 545871.

    
  

        Attachment #426262 -
        Flags: approval1.9.2.2?

        Attachment #426262 -
        Flags: approval1.9.1.9?

        Attachment #426262 -
        Flags: approval1.9.0.19?

    
    

    
  
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. :)

        Attachment #426262 -
        Flags: approval1.9.2.2?

        Attachment #426262 -
        Flags: approval1.9.2.2+

        Attachment #426262 -
        Flags: approval1.9.1.9?

        Attachment #426262 -
        Flags: approval1.9.1.9+

        Attachment #426262 -
        Flags: approval1.9.0.19?

        Attachment #426262 -
        Flags: approval1.9.0.19+

    
    

    
  
Comment on attachment 426262 [details] [diff] [review]
fix v4

This missed the deadline.

        Attachment #426262 -
        Flags: approval1.9.0.19+ → approval1.9.0.19-

    
    

    
  
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).
Keywords: verified1.9.1, 
          
            verified1.9.2
Flags: wanted1.9.0.x+

    
  
Alias: CVE-2010-0181

    
    

    
  
mustlive posted on bugtraq that other browsers are affected http://www.securityfocus.com/archive/1/511327

    
    

    
  
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
Group: core-security

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: