477118 - https webpage with data: images trigger a "Page contains unencrypted information" mixed content warning

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[theglu]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.5) Gecko/2008123017 GranParadiso/3.0.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b3pre) Gecko/20090125 Shiretoko/3.1b3pre

Https webpage with base64 encoded images trigger a "Page contains unencrypted information" with Firefox 3.1. This dosen't happend in Firefox 3.0.

Reproducible: Always

Steps to Reproduce:
Insert this into a https webpage :

<img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAAAXNSR0IArs4c6QAAAAZiS0dEAP8A/wD/oL2nkwAAAAlwSFlzAAAN1wAADdcBQiibeAAAAAd0SU1FB9kBDxIAOdxBIbcAAAI+SURBVDjLxZO7TxRhFMV/M/PN7EPYhS02yytggo+ERAgxERsNhYnGzsQojaHT3r9F0cTaUjs6NUQLBRKDi4I8FEwkLMgO+5rdeX3XAoVYiZWnP797cu69Bv8gqZ+0QTqIdRpl1VHGvnV884CJwQgwicVtoB/ku/H4ycOnURgP/Q1g21Fw4+r0RjbbNmaq4ayO35UkXp9S2UzHxK2bE4QNl82FVwD0D49jn+hERBCRgwTiIX7pvGUGYF/BlO327d3ENfV7wuL0AyjPICIsfPvAmev3qdVqlMtlCoUCGxurDPT1kEnPomQKkRTFpXOjh4DN4kvGLuSJ/JC38y/ouXyXYrGIiBDHMVtbu1SrvXTlbfp6HcQcoVwp5hRAaWedPXFYfL+E1lAKswSRy+joKEEQICLkcjm01hiGgRcksW0b4SMKYHb+OSt6ma+VffxWRF3S5OeeMX7pHkqpwy5EBK31H90ogFODF1lbm8OtlEhloDORpbdnhDAMaTabhwbbtkkkEkRRhGVZR4BCfojJO4+o1VwQAItEIolpmliWRVtbG7s/dlDKotGo47U8CvkuAEwA3/eJoxhE0WyGuO4+nucRBAHVahXDMKg3agRhQBAGVKsVtNZHCRzHwTAMcrnc0eX9it3e3g7A6cGzhx0AKHWwQOV5jdXXb2ZOaa3BOP5ffF5ZptVqflG+3xpbWv5kh2FolEo7dnmv7Lium6zXG0nf95MiguM4QTqd9jPZTCvX2eF393QHqVRKZ7KZkP+unzPGLX8Jr7F8AAAAAElFTkSuQmCC" />

Actual Results:  
trigger a "Page contains unencrypted information" warning.

Expected Results:  
Does not complain as the information is secure..

Works in Fx3.0

Comment 1

[Mardeg]

Attachment: testcase of base64 encoded image

Comment 2

[Eddy Nigg (StartCom)]

It works with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b3pre) Gecko/20090201 Shiretoko/3.1b3pre

Comment 3

[Eddy Nigg (StartCom)]

Sorry, please disregard the comment above. The page has insecure content.

Comment 4

[theglu]

Don't forget to set option in preferences to show this kind of warning btw ;)

Comment 5

[Matthias Versen [:Matti]]

could be related to bug 455367

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Not a blocker; would take a patch.

Comment 7

[Kai Engert (:KaiE:)]

cc'ing Honza who worked on insecure image detection

Comment 8

[Honza Bambas (:mayhemer)]

-> me
going to take a look at this

Comment 9

[Honza Bambas (:mayhemer)]

We cannot detect from securityUI that image proxy request is using data channel to load the image - the underlying request is inaccessible - and image proxy itself cares no security info. I had an idea to let the data channel take security info from document channel of it's doc shell accessed through notification callbacks, but it is no-no because we cannot use nsIDocShell from netwerk code. We could add a new data-channel-specific code to NewImageChannel in imgLoader.cpp to add a security info there but neither libimg has access to doc shell.

So, we can add some new interface to say securityUI to ignore the content in this specific case.

However, it seems to me that we have to do some more work to fix this issue.

Comment 10

[Honza Bambas (:mayhemer)]

Any changes landed here should be covered by mixed content tests from bug 452401.

Comment 11

[Honza Bambas (:mayhemer)]

Attachment: hohoh v1

This is a bit hacky workaround. When URI scheme is data: say we didn't transfer any data from network (what is actually true). That makes the security UI ignore this image request. This is based on bz's fix for one of mixed content image bugs.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Wait.  Why is this the right change?  What about an image from a javascript: URI?  Isn't checking for the URI_INHERITS_SECURITY_CONTEXT flag what you really want?  And even given that, wouldn't you care about _where_ it's inheriting from?

Comment 13

[quantumpacket]

This issue is also occurring with resource: and chrome: image file requests. It is causing partially encrypted errors to occur with NoScript, LinkAlert, and RequestPolicy extensions by causing their icons which are resource: and chrome:

Comment 14

[Boris Zbarsky [:bzbarsky]]

Alternately, should we be checking for LOCAL_RESOURCE, if that's really the criterion we care about?

Comment 15

[Honza Bambas (:mayhemer)]

CrYpTiC MauleR: This has been fixed on trunk and 1.9.1 in bug 450912, please turn your comment there, this is a different bug.

Comment 16

[Honza Bambas (:mayhemer)]

Attachment: v2

(In reply to comment #12)
> Wait.  Why is this the right change?  What about an image from a javascript:
> URI?  Isn't checking for the URI_INHERITS_SECURITY_CONTEXT flag what you really
> want?  And even given that, wouldn't you care about _where_ it's inheriting
> from?

javascript is ignored by security UI.

something like this?

Comment 17

[Boris Zbarsky [:bzbarsky]]

> javascript is ignored by security UI.

Yes, I'm just saying the mechanism for ignoring these should be the the same, and use properties of the protocol, not be hack upon hack of scheme special-casing.

Comment 18

[Boris Zbarsky [:bzbarsky]]

I like "wip" more.

Is there a reason we let file:// URIs affect security UI but don't let resource:// URIs do it?

Comment 19

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 365950 [details] [diff] [review]
hohoh v1

This is certainly not what we want, imo.

Comment 20

[Honza Bambas (:mayhemer)]

(In reply to comment #18)
> Is there a reason we let file:// URIs affect security UI but don't let
> resource:// URIs do it?

Files are also ignored, but not by checking a flag or a uri scheme but by QI of the request to nsIFileChannel.

Comment 21

[Honza Bambas (:mayhemer)]

Comment on attachment 366002 [details] [diff] [review]
v2

This fixes the problem and doesn't break regression tests we have so far. As defined, URI_INHERITS_SECURITY_CONTEXT says that requests with this kind of uri has the same security level as the document, so we may presume there is no need to consider such requests as potentially breaking security ui state. Bz, good idea.

Asking also Kai for review to let him check this.

Comment 22

[Honza Bambas (:mayhemer)]

Comment on attachment 365950 [details] [diff] [review]
hohoh v1

bz found a better approach in a different code.

Comment 23

[Boris Zbarsky [:bzbarsky]]

How about we just check for the LOCAL_RESOURCE flag then?  That would cover resource, chrome, data, file, moz-icon, and anno protocols, and any extension protocol that's correctly written.  So you won't have to keep adding special-cases and _still_ break the moment an extension is in use...

Comment 24

[Honza Bambas (:mayhemer)]

Attachment: v3

(In reply to comment #23)
> How about we just check for the LOCAL_RESOURCE flag then?  That would cover
> resource, chrome, data, file, moz-icon, and anno protocols, and any extension
> protocol that's correctly written.  So you won't have to keep adding
> special-cases and _still_ break the moment an extension is in use...

It works for me. I don't include the URI_INHERITS_SECURITY_CONTEXT check because it's related only to wyciwyg requests that are handled specifically by the security UI code.

Comment 25

[Honza Bambas (:mayhemer)]

And, I will add a test for this bug after bug 480713 gets fixed and mixed content tests are re-enabled.

Comment 26

[Boris Zbarsky [:bzbarsky]]

URI_INHERITS_SECURITY_CONTEXT has nothing to do with wyciwyg.  wyciwyg is document.write().  URI_INHERITS_SECURITY_CONTEXT is javascript: and data:.

As long as we sanely handle javascript: through some other method, patch looks good go me.

Comment 27

[Honza Bambas (:mayhemer)]

Javascript is also handled separately, so it should be ok.

Comment 28

[Boris Zbarsky [:bzbarsky]]

So wait.  With this patch, can we remove all that QI special-casing of channels we have?  As a followup bug or something?

Comment 29

[Honza Bambas (:mayhemer)]

We could consider it, but what I wanted to say is that the isSubDocumentRelevant flag we set false for local resources is not relevant for wysiwyg because it is handled separately. So is javascript.

(In reply to comment #26)
> URI_INHERITS_SECURITY_CONTEXT has nothing to do with wyciwyg.

http://mxr.mozilla.org/mozilla-central/source/content/html/document/src/nsWyciwygProtocolHandler.cpp#128

Comment 30

[Boris Zbarsky [:bzbarsky]]

Oh, huh.  I'd forgotten this part.  Those URIs never really inherit a security context, so...

Comment 33

[[e-mail]]

Hello, i have a similar problem, and i think that maybe its related to this bug. Heres my bugreport: https://bugzilla.mozilla.org/show_bug.cgi?id=498376

Comment 34

[Denny Crane]

Bug still exists in 3.5RC2. The following in a linked CSS file triggers the bug:
background-image: url(data:image/gif;base64,AAAA);

Comment 40

[jonbugzilla]

Is there an eta on getting this fixed?  I'd be happy to be of any assistance.

Comment 41

[Honza Bambas (:mayhemer)]

(In reply to comment #40)
> Is there an eta on getting this fixed?  I'd be happy to be of any assistance.

Depends on Kai's or someones review. I'll try to find someone else to do it.

Comment 42

[Honza Bambas (:mayhemer)]

Comment on attachment 366929 [details] [diff] [review]
v3

Robert, could you please take a look? However, it's not that simple to review...

Comment 43

[Robert Relyea]

Comment on attachment 366929 [details] [diff] [review]
v3

The change looks reasonable, but I am unfamiliare with the URI handle to know if the constant change is the correct one, you probably need a different reviewer.

bob

Comment 44

[Xan Charbonnet]

Any chance of this being fixed in a 3.5.x release?  Right now I'm having to tell my customers to wait to upgrade to 3.5 because of this.  I really don't want to have to tell them they'll never be able to.

Comment 45

[sjs]

I'll +1 this one.  This is causing the bogus security warning for my add-on, as well as many others:

http://www.google.com/search?q=%22contains+unauthenticated+content%22+firefox+add-on

Very unsettling for end users.

Comment 46

[Xan Charbonnet]

This is a serious security flaw, because it destroys the value of that important error message, as everyone gets trained to ignore it.

I realize users are trained to click OK to just about anything already, but that should be getting better, not made worse, especially not by a case of "crying wolf" in a security matter.

Comment 47

[Allen Castaban]

I completely agree. I am holding off updating to 3.5 because of this. It should be a priority

Comment 48

[Johnathan Nightingale [:johnath]]

(In reply to comment #46)
> This is a serious security flaw, because it destroys the value of that
> important error message, as everyone gets trained to ignore it.
> 
> I realize users are trained to click OK to just about anything already, but
> that should be getting better, not made worse, especially not by a case of
> "crying wolf" in a security matter.

This bug doesn't need "me too" advocacy. No one disputes that it should be fixed.  What this bug actually needs...

(In reply to comment #25)
> And, I will add a test for this bug after bug 480713 gets fixed and mixed
> content tests are re-enabled.

is an updated patch with tests for review. Honza - I would personally expect that for this change, someone like bz might be a good alternate reviewer?

Comment 49

[Honza Bambas (:mayhemer)]

(In reply to comment #48)
> Honza - I would personally expect
> that for this change, someone like bz might be a good alternate reviewer?

Yes, I will create a test for this and add a new patch, then I'll ask bz for review.

Comment 50

[Honza Bambas (:mayhemer)]

Attachment: v3 + test [Checkin comment 52] [Checkin 1.9.1 comment 63] [Checkin 1.9.2 comment 64]

Comment 51

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 394815 [details] [diff] [review]
v3 + test [Checkin comment 52] [Checkin 1.9.1 comment 63] [Checkin 1.9.2 comment 64]

I suppose we can live with this....

Comment 52

[sjs]

I apologize for the noob comment, but does the approval1.9.2? flag mean this is being proposed for 1.9.2?  Is there any reason not to propose that it be added to 1.9.1.x given that it seems to be a substantial security concern?

Comment 53

[Honza Bambas (:mayhemer)]

Comment on attachment 394815 [details] [diff] [review]
v3 + test [Checkin comment 52] [Checkin 1.9.1 comment 63] [Checkin 1.9.2 comment 64]

http://hg.mozilla.org/mozilla-central/rev/bec02958a3e1

Comment 54

[Allen Castaban]

If "This bug doesn't need "me too" advocacy. No one disputes that it should be
fixed.", why is not being fixed yet? It was reported in Feb, now it is September, this is over 6 months and several hotfixes old... This is getting quite frustrating...

Comment 55

[Boris Zbarsky [:bzbarsky]]

It's being fixed.  The fix is in the development builds. It's being tested.  Once it's been satisfactorily tested, it will be approved for the upcoming Firefox 3.6 release.

At the moment, no one has requested approval to land this on 1.9.1 (Firefox 3.5.x).  Need driver triage here to see whether it's even wanted there.

If your question is why it took so long to fix, it's because it's not particularly simple to fix and people have other priorities too.

Comment 56

[Johnathan Nightingale [:johnath]]

(In reply to comment #54)
> If "This bug doesn't need "me too" advocacy. No one disputes that it should be
> fixed.", why is not being fixed yet? It was reported in Feb, now it is
> September, this is over 6 months and several hotfixes old... This is getting
> quite frustrating...

It also doesn't need entitlement. Firefox is built largely by volunteers and
even those paid to work on it charge nothing for the product or its support. If
you want any bug in Firefox fixed, please feel free to attach a patch. If
coding isn't your thing, I submit to you that other forms of help (regression
range finding, test reducing, running development nightly builds) are
substantially superior to folding your arms and insisting that, damnit, your
bug is most important and you'll not tolerate these volunteers working on other
bugs first.

As it happens, Honza has fixed this bug on our mozilla-central development
branch and is now awaiting approval to land it on the release branch as well.
I'm sure he will comment here once he has done so.

Any time you want to help fix a bug instead of complaining about the
insufficiency of the speed with which other people leap to your assistance, we
have an ample backlog. This is an open source project devoted to making the
internet a better place. We're not sitting on our hands, cackling about the
torment we cause our users, nor do we have our fingers up our nose waiting for
helpful souls to guide us as to which bug we should fix next. I'm sorry if I
seem harsh, but my hope is to turn your evident energy for the project in more
positive directions, because right now you're sort of seeming like an entitled
dickhead, and I'm pretty sure that's not your intent.

Comment 57

[Allen Castaban]

Look, I understand this is an open source project and done by volunteers and I am highly grateful on that.
I am highly security conscious person and this effects FF security. How do I know whether the page has insecure content or not if I get warning for every SSL page I visit? (And yes certain people will get that for every SSL page they visit, irregardless of mixed content or not). One of my primary reasons I use FF is I find it is a lot securer. And that is one of the primary reasons I recommend to my friends. This problem does no bode well with that and it reduces FF usage. I don't understand why this is not scheduled for 3.5.x?  After all it is a security flaw. When will 3.6 come out?

And  Johnathan, this is not MY bug, it was reported by somebody else and it is effecting plenty other people. If you do not understand my frustration on a security issue reported 7 months ago and not fixed, you have a problem. I am involved with Firefox AS MUCH AS I CAN. My opening bug reports proves that. It definitely does not help you calling people ****

Comment 58

[sjs]

I'd hate to see this turn into a flame war, especially because I think it would distract from getting this important bug fixed.

Johnath, I tried to ask whether this can make it into 1.9.1 here:
https://bugzilla.mozilla.org/show_bug.cgi?id=477118#c52

But frankly I don't understand the bugzilla process well enough to know how to do this formally.  I'm also happy to help with testing etc, but I'm such a noob to the process I don't know where to start.

Comment 59

[Daniel Veditz [:dveditz]]

Landed on trunk so status is "fixed". Still need approval to land on 1.9.2 (triagers there were probably looking for trunk-fixed bugs and skipping this one).

Comment 60

[Honza Bambas (:mayhemer)]

Comment on attachment 394815 [details] [diff] [review]
v3 + test [Checkin comment 52] [Checkin 1.9.1 comment 63] [Checkin 1.9.2 comment 64]

This patch clearly applies to 1.9.1 as well.

Comment 61

[Daniel Veditz [:dveditz]]

Comment on attachment 394815 [details] [diff] [review]
v3 + test [Checkin comment 52] [Checkin 1.9.1 comment 63] [Checkin 1.9.2 comment 64]

This means a file: uri will also not cause mixed-mode, but on windows a file: uri can be a UNC path that goes out over the network.

Comment 62

[Daniel Veditz [:dveditz]]

Comment on attachment 394815 [details] [diff] [review]
v3 + test [Checkin comment 52] [Checkin 1.9.1 comment 63] [Checkin 1.9.2 comment 64]

Approved for 1.9.1.4, a=dveditz for release-drivers

Comment 63

[Honza Bambas (:mayhemer)]

Comment on attachment 394815 [details] [diff] [review]
v3 + test [Checkin comment 52] [Checkin 1.9.1 comment 63] [Checkin 1.9.2 comment 64]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9d30d729f7f6

Comment 64

[Honza Bambas (:mayhemer)]

Comment on attachment 394815 [details] [diff] [review]
v3 + test [Checkin comment 52] [Checkin 1.9.1 comment 63] [Checkin 1.9.2 comment 64]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/fa380bc4f892

Comment 66

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

Verified fixed for 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.4pre) Gecko/20090930 Shiretoko/3.5.4pre using attached testcase.

Comment 68

[Allen Castaban]

I just installed 3.5.4 and I am very happy to report that this bug is now fixed. Thanks to everyone who helped

Comment 69

[dev002]

Do not fixed yet.
Try load data image encoded from external css file under ssl.

Comment 70

[jonbugzilla]

Can someone confirm that 3.5.4 was the first version that has this fix?  And does anyone know in what version this bug first appeared?

Comment 71

[Boris Zbarsky [:bzbarsky]]

> Can someone confirm that 3.5.4 was the first version that has this fix? 

Yes.

> And does anyone know in what version this bug first appeared?

3.5.0.

Comment 72

[Boris Zbarsky [:bzbarsky]]

Unless you were asking for the specific 3.5 alpha/beta/nightly/whatever that first had the behavior?

Comment 73

[jonbugzilla]

Hi Boris,

    Thanks for the quick reply.  The original poster on this thread reported this problem in ff3.1.  Are you sure it didn't appear until 3.5.0?

-jon

Comment 74

[Boris Zbarsky [:bzbarsky]]

The release after 3.0 was 3.5.0.  "3.1" was its internal version number in the early stages, before 3.0 even shipped.

Comment 75

[jonbugzilla]

Hi Boris,

    That clears things up.  So in which alpha/beta/nightly version did it first appear?

Thanks,

-jon

Comment 76

[Boris Zbarsky [:bzbarsky]]

When bug 135007 landed, which puts you at Gecko 1.9.1a2 (aka Firefox "3.1a2") or the 2008-09-02 nightly.

Not sure why that matters, though...

Comment 78

[Onno]

I still have this bug. For example this site on Mozilla Developer: https://developer.mozilla.org/devnews/index.php/2010/03/03/mozilla-developer-preview-now-available-with-out-of-process-plugins/

But of course this is not the only one.

Comment 79

[Allen Castaban]

Because that site contain a mixture of secure and unsecured content. You are supposed to have the warning...

Comment 80

[Onno]

I have also other sites who should be allright but does now and then give that warning and sometimes does not. Even here on bugzilla.

Comment 81

[Black_Ps`]

Sometimes here i have the same problem and on Yahoo login page and when i Refresh it's secured again,it's not happening all the time for me,but i can confirm this old bug.

Comment 82

[Boris Zbarsky [:bzbarsky]]

If what you're seeing is intermittent, it is NOT this bug (which happened 100% reliably).  Please file a new bug with steps to reproduce (e.g. the exact pages you see it happening on) and cc me on it.

Comment 83

[Onno]

I'll file a new bug today.

Comment 84

[Onno]

I filed a bug: https://bugzilla.mozilla.org/show_bug.cgi?id=550580