333428 - Fix for Bug 293527 can be circumvented

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[moz_bug_r_a4]

<img id="i" src="a.png">
  i.src = "http://otherdomain/x.exe";

If the "Load Images for the originating web site only" option is enabled, |src|
property is changed to the new value, but imgIRequest object is not changed,
and |request && (request.imageStatus & request.STATUS_SIZE_AVAILABLE)|
condition is still true.

Thus, "Save Image As..." item appears on the context menu, and it works for the
new |src| value.

I've tested with gavin's patch v3 for Bug 333305.  I think the following change
could fix this.

-        if (this.onImage && (this.target instanceof nsIILC)) {
+        if (this.onImage && (this.target instanceof nsIILC) &&
+            !this.target.imageBlocked) {

Comment 1

[moz_bug_r_a4]

Attachment: testcase

Comment 2

[Daniel Veditz [:dveditz]]

maybe nsImageLoadingContent should null the current request when loading a new one is blocked (near here? http://lxr.mozilla.org/aviary101branch/source/content/base/src/nsImageLoadingContent.cpp#405)

At this point on the old branch I wouldn't want to worry about side effects of that elsewhere, I think your suggestion is safer for now.

Comment 3

[Daniel Veditz [:dveditz]]

> If the "Load Images for the originating web site only" option is enabled,

Note that this is not required for this testcase nor this attack. Changing the src to a non-image such as the executable in the original bug 293527 is all you need to fool us.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

moz_bug: your suggested change was checked in as part of bug 333305. I guess we can mark this fixed now?

Comment 5

[moz_bug_r_a4]

(In reply to comment #4)
> I guess we can mark this fixed now?

Probably yes.  (Today I don't have enough time for more testing.)


(In reply to comment #3)
> > If the "Load Images for the originating web site only" option is enabled,
> 
> Note that this is not required for this testcase nor this attack. 

I cannot reproduce this when "Load Images for the originating web site only"
option is disabled.

When that option is disabled, a non-image file is not blocked, and <img>
element becomes broken image.  Thus, "Save Image As..." item does not appear on
the context menu.

Comment 6

[Jay Patel [:jay]]

For what it's worth, I don't see the "Save Image As..." option in the context menu  at all with the testcase, doesn't matter whether the "originating web site only" pref is set or not.  I guess that means this is "fixed", eh?  

Comment 7

[Jay Patel [:jay]]

I see the same behavior with the latest respins on Windows and Linux (4/10 ~5pm).

Comment 8

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #6)
> For what it's worth, I don't see the "Save Image As..." option in the context
> menu  at all with the testcase, doesn't matter whether the "originating web
> site only" pref is set or not.  I guess that means this is "fixed", eh?

Yeah, it should match 1.5.0.2 (and current trunk/1.8 branch) behavior. The fix was essentially to prevent "Save as" from appearing with the testcase.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

I think we can mark this fixed-aviary1.0.8, fixed1.7.13.

Comment 10

[Jay Patel [:jay]]

Making verified-aviary1.0.8 based on testing of the latest respins.