Closed Bug 1484753 (CVE-2018-12403) Opened 2 years ago Closed 2 years ago

Firefox doesn't mark sites with http favicons as mixed content

Categories

(Firefox :: Security, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 63
Tracking Status
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox61 --- wontfix
firefox62 --- wontfix
firefox63 --- fixed

People

(Reporter: yigitcnyilmaz, Assigned: mossop)

Details

(Keywords: sec-low, Whiteboard: [post-critsmash-triage][adv-main63+])

Attachments

(2 files)

Attached video proof.mov
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.106 Safari/537.36

Steps to reproduce:

1- Open the Firefox
2- Go this website : https://mixed-favicon.badssl.com/




Actual results:

Although a web page icon loads as http, it may appear secure.

proof of concept:
proof.mov


Expected results:

Please look at the https://mixed-favicon.badssl.com's source code. You can see favicon icon as https. But if you open favicon file, it' redirecting to http page. This means : favicon file loading as http. This means : web page is not secure. But Firefox show web page as secure
I can't reproduce this, the mixed content UI shows fine for me. What version of Firefox are you using?
Flags: needinfo?(yigitcnyilmaz)
61.0.2
Flags: needinfo?(yigitcnyilmaz)
Hmm, seems like this got fixed in Nightly, I can reproduce on Beta and Release as well. Would you mind testing this in Firefox Nightly and see if the issue is fixed for you?

Thanks!
Flags: needinfo?(yigitcnyilmaz)
Hello,
Yes. Fixed with nightly version. But I can reproduce on Firefox iOS. Can i create a different report for this ?

Thanks!
Flags: needinfo?(yigitcnyilmaz)
(In reply to Yiğit Can YILMAZ from comment #4)
> Hello,
> Yes. Fixed with nightly version. But I can reproduce on Firefox iOS. Can i
> create a different report for this ?
> 
> Thanks!

Yes, it's preferable to create another bug for Firefox for iOS. Bugzilla has a separate component for this. Thanks!

I'm closing this bug as worksforme then, I'm not removing the security flag until we know that the bug that resolved this isn't s-s either.

Maybe jkt or ckerschb know what fixed this.
Status: UNCONFIRMED → RESOLVED
Closed: 2 years ago
Component: Untriaged → Security
Resolution: --- → WORKSFORME
Summary: firefox http on https → Firefox doesn't mark sites with http favicons as mixed content
I expect this will have been fixed by Mossop's changes to favicon loading. We may want to mark this bug fixed and set the relevant affected/fixed versions.
Flags: needinfo?(dtownsend)
(In reply to :Gijs (he/him) from comment #6)
> I expect this will have been fixed by Mossop's changes to favicon loading.
> We may want to mark this bug fixed and set the relevant affected/fixed
> versions.

My changes didn't land until 63 though and this claimed to have been resolved in 61. Do we have a test in tree for this case? It's possible my change may have affected this.
Flags: needinfo?(dtownsend) → needinfo?(jhofmann)
Hello,
this problem can be reproduced in 61.0.2. If you want to demonstrate you can see "proof.mov" in attachments. But this problem can not be reproduced in the "nightly" version. I can not reproduced in the nightly version. Also, this problem is reproducible for firefox iOS(13.1). I created a new report for iOS . You can see https://bugzilla.mozilla.org/show_bug.cgi?id=1484916 .

Best Regards,
Yiğit
Flags: needinfo?(jhofmann)
Version: 61 Branch → unspecified
(In reply to Dave Townsend [:mossop] from comment #7)
> (In reply to :Gijs (he/him) from comment #6)
> > I expect this will have been fixed by Mossop's changes to favicon loading.
> > We may want to mark this bug fixed and set the relevant affected/fixed
> > versions.
> 
> My changes didn't land until 63 though and this claimed to have been
> resolved in 61. Do we have a test in tree for this case? It's possible my
> change may have affected this.

To clarify, this is fixed in Nightly (63) and broken in 61 and 62, so that sounds a lot like your patch fixed it.

I don't think we have a test for it yet, since that test would have been broken until your change. It would be a good idea to write one.
(In reply to Yiğit Can YILMAZ from comment #11)
> Hello,
> Can you fix it for firefox iOS ? No one responded to my "firefox for iOS"
> report

Unfortunately no-one involved in this conversation is an iOS engineer. I'm afraid you just need to wait till someone from the iOS side of things looks into your report.
Let's use this to get a test landed.
Assignee: nobody → dtownsend
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: WORKSFORME → ---
Comment on attachment 9002841 [details]
Bug 1484753: Loading an insecure favicon should make the page show as mixed content. r=johannh

:Gijs (he/him) has approved the revision.
Attachment #9002841 - Flags: review+
Comment on attachment 9002841 [details]
Bug 1484753: Loading an insecure favicon should make the page show as mixed content. r=johannh

Johann Hofmann [:johannh] has approved the revision.
Attachment #9002841 - Flags: review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/1cba447409917569d3d44d814fd13ba2e6244006
https://hg.mozilla.org/mozilla-central/rev/1cba44740991
Group: firefox-core-security → core-security-release
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
This sounds like a sec-low that's riding the trains, but feel free to correct that if I'm misunderstanding.
I agree with that assessment.
(And there may be a public dupe of this somewhere).
(In reply to Yiğit Can YILMAZ from comment #21)
> Can you give me bug bounty ? and can you add me hall of fame ?

Our bug bounty program is described at https://www.mozilla.org/en-US/security/client-bug-bounty/

I'll go ahead and tag this bug for evaluation but sec-low bugs generally don't qualify.
Flags: needinfo?(dtownsend) → sec-bounty?
The bounty committee agrees that this does not qualify for a reward
Flags: sec-bounty? → sec-bounty-
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main63+]
Alias: CVE-2018-12403
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.