The default bug view has changed. See this FAQ.

Security warning (viewing_mixed) shown on secure page which has images with javascript protocol

VERIFIED FIXED in Firefox -esr10

Status

()

Core
Security: PSM
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: ypetrov, Assigned: bz)

Tracking

({regression})

Trunk
mozilla13
regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr1013+ verified)

Details

(Whiteboard: [qa+:ashughes])

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
Created attachment 592390 [details]
Warning screenshot

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0.1) Gecko/20100101 Firefox/9.0.1
Build ID: 20111220165912

Steps to reproduce:

Enter https://mail.yandex.com.


Actual results:

"Security Warning" window appear (screenshot attached)
Traffic sniffers (Firebug, Charles, SmartSniff and Wireshark) doesn't capture any http requests, there are https and 443 port only.
Our guess is that CORS long polling request to https://webchat2.yandex.com triggers this warning. However blank page with that request doesn't reproduce that behaviour.


Expected results:

No "Security Warning" on these pages.
Also some debugging information on these warnings would be very helpful.
(Reporter)

Comment 1

5 years ago
Created attachment 592391 [details]
Sniffer screensot
(Reporter)

Updated

5 years ago
Attachment #592390 - Attachment description: Снимок экрана 2012-01-27 в 16.32.57.png → Warning screenshot
(Reporter)

Comment 2

5 years ago
Steps to reproduce (updated):

1. Register and login at https://mail.yandex.com.
2. Go to Settings - Chat.
3. Check "Enable chat in Yandex.Mail".
4. "Save changes".

Comment 3

5 years ago
Related to or dupe of bug 506008?
(Reporter)

Comment 4

5 years ago
No. There are no redirects from http to https.
Also this behaviour introduced in FF9. FF8 doesn't show this warning.
(Reporter)

Updated

5 years ago
Summary: Security warning (viewing_mixed) shown on secure page → Security warning (viewing_mixed) shown on secure page which has images with javascript protocol
(Reporter)

Comment 5

5 years ago
We've found what have triggered this warning. There is blank image with src="javascript:null" which considered non-secure.

Test page: https://locum.ru/YgpbrN1n7g/

Comment 6

5 years ago
Using testcase in comment 5:

WFM:
Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.2.25) Gecko/20111212 Firefox/3.6.25
Opera/9.80 (X11; Linux x86_64; U; en) Presto/2.10.229 Version/11.61
Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/535.7 (KHTML, like Gecko) Chrome/16.0.912.77 Safari/535.7

Reproduced:
Mozilla/5.0 (X11; Linux x86_64; rv:9.0.1) Gecko/20100101 Firefox/9.0.1
Mozilla/5.0 (X11; Linux x86_64; rv:10.0) Gecko/20100101 Firefox/10.0
Mozilla/5.0 (X11; Linux x86_64; rv:11.0a2) Gecko/20120129 Firefox/11.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:12.0a1) Gecko/20120130 Firefox/12.0a1


Last good nightly: 2011-09-21
First bad nightly: 2011-09-22

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e8bd19f6abbb&tochange=4495e1f795c2
Keywords: regression
OS: Mac OS X → All
Hardware: x86 → All
Version: 9 Branch → Trunk
Maybe bug 444641?

Comment 8

5 years ago
The first bad revision is:
changeset:   77221:c237a8550070
user:        Boris Zbarsky <bzbarsky@mit.edu>
date:        Tue Sep 20 17:00:42 2011 -0400
summary:     Bug 444641 part 3.  Propagate the loading principal to the image channel as needed.  r=joe,dveditz

https://hg.mozilla.org/mozilla-central/rev/c237a8550070
Blocks: 444641
Status: UNCONFIRMED → NEW
Component: Untriaged → General
Ever confirmed: true
Product: Firefox → Core
QA Contact: untriaged → general
That's ... interesting.  What used to happen there is that we didn't propagate a principal, so the javascript: URI never ran and the image never got loaded at all.

Now we _do_ load the image.  So now we get progress notifications for it.  nsSecureBrowserUIImpl::OnStateChange checks for and ignores such notifications for javascript: URIs, but only if the incoming request is an nsIChannel.  For images it would be an imgIRequest.  The javascript: check should probably move to after the imgIRequest block.
Component: General → Security: PSM
QA Contact: general → psm
Created attachment 595312 [details] [diff] [review]
Correctly ignore javascript: images in security UI.
Attachment #595312 - Flags: review?(kaie)
Assignee: nobody → bzbarsky
Whiteboard: [need review]
Oh, two more things:

1)  I really appreciate the problem diagnosis and simple testcase!
2)  Kai, how do I add a test for this?

Comment 12

5 years ago
> 2)  Kai, how do I add a test for this?

Honza wrote all the tests for the secure page state that we have.
They are in security/manager/ssl/tests/mochitest/mixedcontent.

I assume you want the test to be:
- Have a https page
- page contains <img src="javascript:null">
- test that the security state is still "secure"

I have not played with the tests myself. I looked briefly, and test_securePicture.html could be a good minimal starting point for the current need. At the top of mixedContentTest.js is a helpful comment.

From my reading of the comment, I'd guess we need
  var bypassNavigationTest = true;

and 

  function runTest()
  {
    isSecurityState("secure", "image with null javascript is secure");
    finish();
  }

Comment 13

5 years ago
Comment on attachment 595312 [details] [diff] [review]
Correctly ignore javascript: images in security UI.

Looks good to me, but I haven't tested it. Thanks for your help.

r=kaie
Attachment #595312 - Flags: review?(kaie) → review+

Comment 14

5 years ago
Because this bug is a regression in shown security state, it should be considered for the extended support branch.
tracking-firefox-esr10: --- → ?

Updated

5 years ago
Whiteboard: [need review]
> They are in security/manager/ssl/tests/mochitest/mixedcontent.

Perfect, that's what I needed.  I have a test for this now.
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0a7f8c60ccc
Flags: in-testsuite+
Target Milestone: --- → mozilla13
https://hg.mozilla.org/mozilla-central/rev/d0a7f8c60ccc
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
[Triage Comment]
tracking for ESR Firefox 13
tracking-firefox-esr10: ? → 13+
Comment on attachment 595312 [details] [diff] [review]
Correctly ignore javascript: images in security UI.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Suggested by PSM module peer.
User impact if declined: Some secure pages are flagged as insecure in the UI
Fix Landed on Version: Firefox 13.
Risk to taking this patch (and alternatives if risky): Low risk, I believe.
   Should only affect image loads from javascript: URIs.
String changes made by this patch:  None.

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #595312 - Flags: approval-mozilla-esr10?
Comment on attachment 595312 [details] [diff] [review]
Correctly ignore javascript: images in security UI.

approving for esr landing based on improving the user experience with images in security UI
Attachment #595312 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
https://hg.mozilla.org/releases/mozilla-esr10/rev/0a153b10080b
status-firefox-esr10: --- → fixed
@ypetrov, can you please confirm this is fixed in latest-mozilla-esr10?
ftp://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-esr10/
Whiteboard: [qa+]
Verified fixed with Firefox 10.0.5esrpre 2012-05-31.
Status: RESOLVED → VERIFIED
status-firefox-esr10: fixed → verified
Whiteboard: [qa+] → [qa+:ashughes]
You need to log in before you can comment on or make changes to this bug.