331257 - data: images show up when 'load images' (Tools->Options->content->checkbox) is disabled

Status: NEW

(Core :: Graphics: Image Blocking, defect)

Description

[Adam]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1

It is unclear whether this is intended; the option to turn off 'load images' in the preferences does not halt the rendering of data: url images. Arguably the option above is only meant to disable loading of images external to the HTML/XHTML file, but would it then be a future addition to have a separate option to exclude data: images?

Reproducible: Always

Steps to Reproduce:
1.Turn off the option (Tools->Options->'content'->uncheck 'load images' checkbox)
2.Go to the above URL or any with a data: image included
Actual Results:  
Data: Image shows up as normal

Expected Results:  
Depending, not show up, or if the option is not intended to disable data: images, it works fine.

Marked as trivial. Would be good to have a solution in mind, and eventually implemented, but this is not critical by any means given especially that data: images are infrequently used at all.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Yeah.. the problem is this code in nsContentBlocker::ShouldLoad:

166   // we only want to check http, https, ftp
167   // for chrome:// and resources and others, no need to check.
168   nsCAutoString scheme;
169   aContentLocation->GetScheme(scheme);
170   if (!scheme.LowerCaseEqualsLiteral("ftp") &&
171       !scheme.LowerCaseEqualsLiteral("http") &&
172       !scheme.LowerCaseEqualsLiteral("https"))
173     return NS_OK;

Maybe we should use a URI flag here instead?  And set it on more protocols?

Note that this is not restricted to data: -- ftp: , gopher:, etc could have similar problems.

Comment 2

[Damon Sicore (:damons)]

Boris, still think this should block?

Comment 3

[Boris Zbarsky [:bzbarsky]]

Well...  it depends on why we have this preference.  If we're OK with it not working in a bunch of cases, then it shouldn't.

I do think this should be wanted, at least.

Comment 4

[AndrewM]

Version should be changed to Trunk, and if it is an issue on all platforms (I can only test with Windows Vista), Hardware and OS to All too.

Comment 5

[Damon Sicore (:damons)]

-'ing and marking as wanted per boris' comment.

Comment 6

[Vasil Dinkov]

This is still confirmed in FF 3.1b2 on WinXP.

I believe when images are disabled by the user, data URI images should NOT be loaded because the user intends to disable all images on the pages with this option (no matter what their source is).

Also all other browsers do not load data URI images when images are disabled by the user (only FF loads them).

Comment 12

[Adam]

Attachment: Rely on URI flags as suggested in comment 1

Everything we want to consider blocking, except the file protocol, has the flag nsIProtocolHandler::URI_LOADABLE_BY_ANYONE.  file is the only protocol with URI_IS_LOCAL_FILE, so a separate check catches that.

This also adjusts nsContentBlocker::TestPermission to allow data URIs (which would now be blocked) for the BEHAVIOR_NOFOREIGN case (permissions.default.image == 3).

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 615568 [details] [diff] [review]
Rely on URI flags as suggested in comment 1

This is still hardcoding data: in nsContentBlocker, no?  This code will break with various other URI schemes, no?

Comment 14

[Adam]

Attachment: Uses a URI flag to check for context-citizenship of data: when third-party images are disabled

The ShouldLoad() change works with all schemes.  We only consider blocking images in content (be it from local files, from data:, or from some network protocol).  The TestPermission() change was needed because data: is currently unique in that it ought not be considered a third-party, despite not having the same host as its context.  

data:'s uniqueness could change in the future, so this version replaces the data: hard-coding with a check for a new flag: nsIProtocolHandler::URI_INHERITS_HOST.  This improvement allows new protocols to use that flag and not require modifications to nsContentBlocker.

One possible alternative is using the URI_INHERITS_SECURITY_CONTEXT flag.  It's currently only used by data: and wyciwyg:, but the latter is already excluded in ShouldLoad().  That flag's meaning is different enough than needed here, so the new flag seems safer.  

It should also be noted that the flag doesn't literally cause the URI to inherit the context's host, just to be treated as though it does by nsContentBlocker.  If other code required access to a literal host, that could be changed for data: or other protocols using the flag.

Comment 15

[Boris Zbarsky [:bzbarsky]]

> data:'s uniqueness could change in the future,

It's not unique today.  Extensions can implement new protocol handlers, and quite a number do so.

Comment 16

[Boris Zbarsky [:bzbarsky]]

> One possible alternative is using the URI_INHERITS_SECURITY_CONTEXT flag.

Yes, this is what you should use, imo.  That means it inherits the origin (so not just the host, but also the port and scheme) for purposes of security checks.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Perhaps a better question: if image loading is disabled for a site, should that site be able to load data: images?  That is, what behavior are we actually after?

Comment 18

[Adam]

Attachment: Use the URI_INHERITS_SECURITY_CONTEXT flag, and use the context's permission as a proxy for those protocols

(In reply to Boris Zbarsky (:bz) from comment #17)
> Perhaps a better question: if image loading is disabled for a site, should
> that site be able to load data: images?  That is, what behavior are we
> actually after?

We treat them as though they are from the context, so we use the context's settings as a proxy, so we can avoid changing TestPermission() altogether.  This patch is updated to reflect that.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 616283 [details] [diff] [review]
Use the URI_INHERITS_SECURITY_CONTEXT flag, and use the context's permission as a proxy for those protocols

r=me.  Thanks!

Comment 20

[Dão Gottwald [:dao]]

I fail to see this bug's point. As I understand it, the point of that pref is to save bandwidth. Presumably that's why it says "load" rather than "display".

Comment 21

[Nochum Sossonko [:Natch]]

(In reply to Dão Gottwald [:dao] from comment #20)
> I fail to see this bug's point. As I understand it, the point of that pref
> is to save bandwidth. Presumably that's why it says "load" rather than
> "display".

Or block offensive images, or simplify page perusal, etc. IMHO this is the correct thing to do, especially since most users won't understand the difference between data: images and loaded images and will just think "Firefox is broken". So that would be another reason.

Comment 22

[Dão Gottwald [:dao]]

> Or block offensive images, or simplify page perusal, etc.

Wouldn't be the first time that people wanted to use features for something they weren't intended for. I still believe the reason we have that option is the one I mentioned. It's particularly useful for mobile browsing, where slow connections or bandwidth limits are still common.

Comment 23

[Dão Gottwald [:dao]]

Comment on attachment 616283 [details] [diff] [review]
Use the URI_INHERITS_SECURITY_CONTEXT flag, and use the context's permission as a proxy for those protocols

I think we should close this as WONTFIX. The pref is working as intended.

Comment 24

[lowbatteries]

I would prefer parity with other browsers.

Chrome: "Do not show any images" - no data: images
Safari: "Display images when the page opens" - no data: images
IE: "Show pictures" - no data: images

Opera is the exception here, with two settings: 
"No images" - no data: images 
"Cached images" - DOES display data: images

Comment 25

[Dão Gottwald [:dao]]

(In reply to lowbatteries from comment #24)
> I would prefer parity with other browsers.
> 
> Chrome: "Do not show any images" - no data: images
> Safari: "Display images when the page opens" - no data: images
> IE: "Show pictures" - no data: images

As phrased, this makes sense for them. Our option is labeled differently and I argue it's in line with why most users would disable images.

Comment 26

[Adam]

I think the feature, as it's currently presented to users, is about blocking images in content.  User expectation is that no images will display, which is what this bug is about.

Most users of it want to save bandwidth, but when they do run across a data: image, it is a distraction.  They may pause their activity to evaluate whether their browser is acting correctly.

Comment 27

[Klem]

Have the potential security ramifications of this bug been properly considered?

Apart from making it impossible for users to prevent themselves from being presented with potentially objectionable images, as already discussed, the embedded images must be decoded and rendered. When vulnerabilities are discovered in a browser's image code, isn't the usual first response to warn users to mitigate the bug by disabling images? While that advice would appear sound for every other browser, doesn't this bug present an obvious vector against Mozilla users under such circumstances?

Furthermore, some users who seek to make their browser as secure as possible routinely (attempt to) block images as a precaution against undisclosed "zero day" exploits. The current behaviour/bug would seem to negate their efforts and can come as quite a shock - as illustrated here: https://tails.boum.org/forum/Base64_Encoded_Images:_How_to_block_them__63__/

I can't find any discussion of potential security aspects to this bug yet, but I notice it was reassigned from a security-bugs@... address to [e-mail] (before comment 2). So as at least someone else must have been worried by this I've requested a review by the security team. I hope that's not some awful faux-pas :-B

Comment 28

[David E. Ross]

Re comment #27:  

Images with malware are indeed known.  The US-CERT (an agency of the U.S. Department of Homeland Security) lists many.  Try the following query at the US-CERT Web site:
<http://search.us-cert.gov/search?utf8=%E2%9C%93&input-form=advanced&affiliate=us-cert&query-or=JPEG+GIF+PNG+BMP&per-page=10&filter=off&x=31&y=9>.

Comment 29

[Daniel Veditz [:dveditz]]

The security team doesn't think we need to review this decision from a security POV. Our understanding was that the original intent was to save bandwidth; not wanting to see objectionable content is a different valid reason, but it's a product reason and needs to be made at the product level.

Yes, image formats sometimes have vulnerabilities, but we have to fix those because most people will not discover this feature nor protect themselves using it. If we did decide this pref was actually a defensive measure then we would also need to disable <canvas> should it be flipped, because you can programmatically create images (of some formats, at least) using canvas.

Marking this "sec-review+" to indicate we've looked at the issue, not that we're endorsing this specific patch.

Comment 30

[Hans Hillen]

It's not a security issue, it's an accessibility issue. A user may want to disable images for a variety of reasons, which are none of our business really. A user with cognitive disabilities may simply find images in general to distractive and prefer a text only version of a web page. A user with low vision may want to save layout by stripping out images in an enlarged text view. A user may simply prefer to see alt text rather than the actual image.

Whatever the reasons are, the average user wouldn't have a clue what the difference is between an external image source and data URI image source, it's only us developers that would know this. Since this is a feature intended to improve user experience, it should function in a way that makes sense to that user. Unchecking a feature called "Load images automatically" should mean "disable all images", not "disable most but not all images".

In addition, handling data URI images consistently when images are disabled would make it a lot easier for web developers to detect whether those images are currently disabled or not. Because of this bug, we're forced to load an external sample image in order to be able to detect this in Firefox. I'n all other major browser we can use a data URI img src, which is much easier for this purpose.

The comments for this bug go back to 2006, but I haven't heard a single good reason why disabling images in the browser's user settings SHOULDN'T apply for data URI images as well. I would really appreciate it if it got fixed, if only for the sake of consistency.

Comment 31

[magnus]

I agree with what Hans Hillen said ^^above^^.

It feels like data URI images are a way for them to force images on me that I dont want, and ABP cant block them and neither can NoScript. And I was angry when found out from this thread that other browsers can block data URI images but firefox cant. I have stuck with firefox cuz I cant do without the addons such as ABP and NoScript, but I see that Opera now has addons like firefox so I am thinking about switching to Opera.

I am running across data URI images with increasing frequency.

A web page can have many large data URI images embedded in it making the user download several megabytes of content he does not want.

A bad example of data:image is here http://newworldordernews.com/page4.php I have a slow connection it takes me a coons age to download that page, I want to block all data:image on that page, and that page is not the worst example.

What I would really like is and addon that specifically blocks just the data URI images.

Comment 33

[Eduard R.]

(In reply to magnus from comment #31)
> What I would really like is and addon that specifically blocks just the data
> URI images.

Image Block addon might be what you're looking for.

https://addons.mozilla.org/en-US/firefox/addon/image-block/

Comment 34

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S4. However, the bug has 5 duplicates and 12 votes.
:aosmond, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 35

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

Comment 36

[ed]

(In reply to Hans Hillen from comment #30)

Unchecking a feature called "Load images automatically"
should mean "disable all images", not "disable most but not all images".

Agreed, but more granularity is needed so that phrasing should be changed.

There are two kinds of users in this regard:

1. People on capped or measured rate connections need to keep their network consumption low. For this use case images should not be fetched from the cloud. But this group of users are happy to render images that do not require a separate fetch over the network.

2. People who have unlimited bandwidth but they do not want garbage in the window. The browser can should respect their choice whether the images are embedded or not. This group is satisfied by your comment.

When you use the term URI images, I assume you are talking about embedded base64-ified images like that described by this guide, correct?

The comments for this bug go back to 2006, but I haven't heard a single good
reason why disabling images in the browser's user settings SHOULDN'T apply
for data URI images as well. I would really appreciate it if it got fixed,
if only for the sake of consistency.

¶1 covers this. Someone on a budget connection wants to avoid fetching the image but opts to display what they can.

There’s another scenario for that same use case: email security. People are generally happy to load embedded¹ images to get close to the intended presentation but nothing that uses the cloud because “tracker pixels” are used to track when the message is rendered and approximately where the recipient is located at that time. People with text-based mail clients like Mutt sometimes call Firefox to render HTML messages.

¹: there are two kinds of embedded images AFAIK. One way is the base64 inlined string and the other way is separate files that are attached to the email whereby a filename is given but it sits locally. Both forms of embedding should be treated the same as far as email users are concerned, but WAN images are distrusted.