Closed
Bug 645564
Opened 14 years ago
Closed 14 years ago
"view-source:" on DV SSL does not update site identity button text -> spoofing
Categories
(Firefox :: Security, defect)
Tracking
()
RESOLVED
FIXED
Firefox 6
People
(Reporter: matt, Assigned: Gavin)
References
()
Details
(Whiteboard: [sg:moderate] spoof)
Attachments
(1 file)
8.73 KB,
patch
|
Dolske
:
review+
|
Details | Diff | Splinter Review |
Navigating to a "view-source:" page on a DV SSL site or switching to a tab showing such a page fails to update the text of the site identity button. This can be used to spoof the name of another DV SSL site in the site identity button by opening a new window on that site and then navigating the window to a "view-source:" page on one's own site. See the demo in the URL field.
The site identity button color is updated correctly in all cases, and the text is updated correctly when navigating/switching to "view-source:" on an EV SSL site. Therefore, this bug does not make it possible to spoof text in a green site identity button.
Assignee | ||
Comment 1•14 years ago
|
||
The patch in bug 522319 would fix this, by ensuring that view-source pages have useful location objects (i.e. whose .hostname doesn't return an empty string). That's a different issue, though - at a minimum we should avoid throwing here.
Assignee | ||
Comment 2•14 years ago
|
||
I filed bug 646690 on the general issue of location.hostname not being useful for view-source pages.
Assignee | ||
Comment 3•14 years ago
|
||
Removes an unused variable (missed cleanup from http://hg.mozilla.org/mozilla-central/rev/9649eb8bda37 ) and avoids calling hasMatchingOverride if the hostname is empty (which throws).
Assignee | ||
Updated•14 years ago
|
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
Assignee | ||
Updated•14 years ago
|
blocking2.0: --- → ?
Updated•14 years ago
|
Whiteboard: [sg:moderate] spoof
Reporter | ||
Comment 4•14 years ago
|
||
The patch seems to work. I wonder if it might be prudent to completely clear the site identity button/popup at the beginning in order to fail secure if there are any other unforeseen exceptions.
Assignee | ||
Comment 5•14 years ago
|
||
That's a good idea - my only concern with that is that this code has been optimized to avoid making too many UI changes, because in the past updating these indicators has had a significant impact on page load time (either because we do too much work when the security state changes, or because we get too many redundant "security state changed" notifications, or both).
Updated•14 years ago
|
blocking1.9.1: ? → ---
blocking1.9.2: ? → ---
blocking2.0: ? → -
status1.9.1:
--- → wanted
status1.9.2:
--- → wanted
Comment 6•14 years ago
|
||
Comment on attachment 523174 [details] [diff] [review]
patch
Review of attachment 523174 [details] [diff] [review]:
::: browser/base/content/browser.js
@@ +7634,1 @@
(this._lastLocation.port || 443),
I was pondering if .port can ever be 0 here (bug 347934 not withstanding), and then noticed you're not actually changing this. :) I'd be a bit surprised if a 0 can make it to here, though.
Also I just wanted to test Splinter.
Oh, yay, I can't change review flags here. :/ Oh, wait, I can. Mode-based interfaces FTL. :(
Attachment #523174 -
Flags: review?(dolske) → review+
Assignee | ||
Comment 7•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 6
Assignee | ||
Updated•14 years ago
|
Flags: in-testsuite+
Updated•10 years ago
|
Group: core-security → core-security-release
Updated•10 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•