Closed
Bug 258048
Opened 20 years ago
Closed 20 years ago
Security indicators updated when page finishes load, not when it starts rendering
Categories
(Core :: Security, defect)
Core
Security
Tracking
()
VERIFIED
FIXED
mozilla1.8beta1
People
(Reporter: Mook, Assigned: darin.moz)
References
(Blocks 1 open bug)
Details
(Keywords: fixed-aviary1.0.1, fixed1.7.6, Whiteboard: [sg:fix] need landing)
Attachments
(1 file)
(deleted),
patch
|
jst
:
review+
bzbarsky
:
superreview+
dveditz
:
approval-aviary1.0.1+
dveditz
:
approval1.7.6+
caillon
:
approval1.8b+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040903 Firefox/1.0 PR (NOT FINAL) Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040903 Firefox/1.0 PR (NOT FINAL) When loading a secure page, the security indicates (lock in status bar, yellow address bar for Firefox) is only changed when the page is completely loaded. It may be possible to pretend a site is secure (and, in the case of Firefox, the domain in the status bar is not updated). Reproducible: Always Steps to Reproduce: 1. Go to a secure site 2. Go to a non-secure site that loads slowly Actual Results: All indicators of secure site are kept until the non-secure site finishes loading, after any page content has been rendered Expected Results: The secure site inidicators are removed when any page rendering begins (and, possibly, restored after it finishes if the new site is also secure) It may be possible to spoof a secure site by: 1. Opening a window to the secure site's login page via script 2. Wait for the site to load (via timeouts? Can't seem to do it via event handlers, which is good) 3. Open a non-secure site in the same window (via script). Spoofs the first document. 4. Make sure the second page never finishes loading (possibly via an hidden image with the download being throttled by the server). To the user, all indications are that the second page is secure - and, excluding the address, also indicates that the second document is on the domain of the first document. Proposed fix: Clear the security indicators when page starts loading. (Sorry if this is a dupe - can't seem to find a dupe) Tested on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040903 Firefox/1.0 PR (NOT FINAL) Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.1) Gecko/20040707 (Seamonkey 1.7.1)
Updated•20 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 2•20 years ago
|
||
Sort of the opposite of bug 257308, which has been made public. Opening this bug so we can figure out what really's going on.
Group: security
Updated•20 years ago
|
Assignee: dveditz → darin
Group: security
Updated•20 years ago
|
Whiteboard: [sg:spoof?]
Assignee | ||
Updated•20 years ago
|
Target Milestone: --- → mozilla1.8beta
Updated•20 years ago
|
Whiteboard: [sg:spoof?] → [sg:fix]
Assignee | ||
Updated•20 years ago
|
Status: NEW → ASSIGNED
Flags: blocking1.8b?
Assignee | ||
Updated•20 years ago
|
Flags: blocking1.7.6?
Flags: blocking-aviary1.0.1?
Updated•20 years ago
|
Flags: blocking1.7.6?
Flags: blocking1.7.6+
Flags: blocking-aviary1.0.1?
Flags: blocking-aviary1.0.1+
Updated•20 years ago
|
Flags: blocking1.8b?
Flags: blocking1.8b+
Flags: blocking-aviary1.1?
Flags: blocking-aviary1.1+
Assignee | ||
Comment 4•20 years ago
|
||
This patch builds on jst's patch for bug 277564 by simply updating the lock icon state for any channel in OnLocationChange. The idea being that OnLocationChange corresponds to a change in the URL bar, so it is therefore the right time to affect the lock icon state. One concern with this patch is that it introduces two security dialogs potentially. In the mixed case, we will first tell the user that the content they are seeing is secure, and then when the page finishes loading, we will tell them that, no, it is actually mixed. This is a concern to me, but I think it is a reasonable trade-off. Certainly, if the toplevel document is insecure, then there is no way for the lock icon to transition away from the unlocked state.
Attachment #173823 -
Flags: superreview?(bzbarsky)
Attachment #173823 -
Flags: review?(jst)
Comment 6•20 years ago
|
||
Comment on attachment 173823 [details] [diff] [review] v1 patch Seems reasonable...
Attachment #173823 -
Flags: superreview?(bzbarsky) → superreview+
Comment 7•20 years ago
|
||
I didn't look at this diff yet, but maybe we should make the code never change to a *secure* state before we actually know that it is secure, but we should for sure change to an *insecure* state as soon as we're not sure that it is secure. Would that be tricky in our code?
Updated•20 years ago
|
Whiteboard: [sg:fix] → [sg:fix] need review jst
Assignee | ||
Comment 8•20 years ago
|
||
> I didn't look at this diff yet, but maybe we should make the code never change
> to a *secure* state before we actually know that it is secure, but we should for
> sure change to an *insecure* state as soon as we're not sure that it is secure.
> Would that be tricky in our code?
That's an interesting idea. But, what would you do if navigating from one
https:// link to another? Would you make the URL bar change from yellow to
white during the page load? Consider the case where we are switching sites, but
those sites might be related from the users point of view. What if the second
site is a spoofer? I think that's enough reason to update the lock icon to the
secured state in OnLocationChange. What do you think?
We should probably also verify that the mixed icon warning appears as soon as we
get some mixed content on the page and not once the page finishes loading.
We have to assume that the page never finishes loading when considering how to
tackle this problem.
Comment 9•20 years ago
|
||
Comment on attachment 173823 [details] [diff] [review] v1 patch r=jst, I think this is indeed an improvment over what we've got today.
Attachment #173823 -
Flags: review?(jst) → review+
Comment 10•20 years ago
|
||
Comment on attachment 173823 [details] [diff] [review] v1 patch a=dveditz for 1.7 and aviary1.0.1 branches
Attachment #173823 -
Flags: approval1.8b?
Attachment #173823 -
Flags: approval1.7.6+
Attachment #173823 -
Flags: approval-aviary1.0.1+
Comment 11•20 years ago
|
||
Comment on attachment 173823 [details] [diff] [review] v1 patch a=caillon for 1.8b. Seems like it is a good (not to mention important) fix, and we can do any further work later.
Attachment #173823 -
Flags: approval1.8b? → approval1.8b+
Updated•20 years ago
|
Whiteboard: [sg:fix] need review jst → [sg:fix] need landing
Fix committed on HEAD.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 14•20 years ago
|
||
fixed1.7.6, fixed-aviary1.0.1 shaver: thanks for landing this on the trunk for me! :)
Keywords: fixed-aviary1.0.1,
fixed1.7.6
Comment 15•20 years ago
|
||
Verified Fixed. Going from a secure site to a non-secure site now immediately removes the yellow background and lock icon when the second site begins to render. Aviary 1.0.1 Branch: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20050217 Firefox/1.0 M18b1/Trunk: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b) Gecko/20050217 M176 Branch: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050217
Status: RESOLVED → VERIFIED
Comment 16•20 years ago
|
||
As I worried over in dupe bug 278003 comment 3 the multiple entering/leaving dialogs have come back :-( See bug 145730 regressed by "show lock early" bug 148610 fixed again in bug 154084 Also we should retest bug 154240 and make sure appropriate warnings are given for redirections. Should I file a new bug, reopen this one, or reopen bug 154084?
Updated•20 years ago
|
Group: security
You need to log in
before you can comment on or make changes to this bug.
Description
•