397937 - view source on malware/web forgery error page results in error page in view-source window

Status: VERIFIED FIXED

(Toolkit :: Safe Browsing, defect, P4)

Description

[Dão Gottwald [:dao]]

If you try to view the source of a blocked malware page, the same error page shows up. Maybe that's by design. But then clicking the "request a review" link from the error page loads the target into the very same Source View window, which hardly makes sense.

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

see also bug 273968

Comment 2

[Dão Gottwald [:dao]]

Bug 273968 must be either invalid or wontfix. If you actually can't reach a server, you can't view the source of a page -- so view-source fails, and the error page makes sense again. You could view the source of the error page, but I don't know what that would be useful for. It would only confuse Web developers. Those who are really interested in the Firefox source can find or inspect it elsewhere.

Anyway, what I would expect here is that malware detection is skipped if I ask to view the source of a page.

Comment 3

[Dão Gottwald [:dao]]

If skipping malware detection for view-source is risky (is it?), then at least the link should be fixed. Requesting blocking.

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

morphing a little

Blocking the "view-source" action is by-design, as (apparently) the view-source window does some parsing which could expose it to an attack. At least, so I'm told.

However, showing the same error page in the view-source window seems like the wrong thing to do. Assigning to Johnath and adding ui-wanted.

Comment 5

[Mike Connor [:mconnor]]

I think I'd lean towards disabling the view source command on error pages entirely, it doesn't seem like its useful, and has odd side effects like this.

Comment 6

[Johnathan Nightingale [:johnath]]

Attachment: halfway there

I tinkered with this briefly before prioritizations came in and this dropped down the list.  The attached patch removes the View Source context menu contribution on about: pages just like we already do on other things like images or selected text.

Unfortunately, it doesn't disable it from the View menu.  I'm not sure what the right approach is there.  One option is to just leave it as-is -- this is what selected text does, for instance.  No right-click option, but the menu item is still present and active.  Another option is to find a good way to disable it, but currently that menuitem just observes isImage for enabled/disabled status, which doesn't lend itself very well to overloading for this case.

http://mxr.mozilla.org/mozilla/source/browser/base/content/browser-sets.inc#78

Comment 9

[David Gerard]

@Mike Connor - "I think I'd lean towards disabling the view source command on error pages entirely, it doesn't seem like its useful, and has odd side effects like this."

Er, it would be very useful, actually, when trying to clean up problems. reddragdiva.co.uk just got hit by the latest SQL injection vulnerability in Coppermine, and I wanted to "view source" to see just what the malware StopBadware was seeing was. I couldn't in Minefield. I had to do it in Konqueror. Making people switch browsers wasn't considered a good move in bug 422410 (option to view a malware page) - it's not clear why it would be a good move here.

Comment 10

[Johnathan Nightingale [:johnath]]

(In reply to comment #9)
> @Mike Connor - "I think I'd lean towards disabling the view source command on
> error pages entirely, it doesn't seem like its useful, and has odd side effects
> like this."
> 
> Er, it would be very useful, actually, when trying to clean up problems.
> reddragdiva.co.uk just got hit by the latest SQL injection vulnerability in
> Coppermine, and I wanted to "view source" to see just what the malware
> StopBadware was seeing was. I couldn't in Minefield. I had to do it in
> Konqueror. Making people switch browsers wasn't considered a good move in bug
> 422410 (option to view a malware page) - it's not clear why it would be a good
> move here.

I sympathize with what you're saying here - certainly it is in everyone's best interests for site operators to find badness as quickly as possible.  However, any attempt to load the malware page, even in view source, should be perceived as basically saying "bring it on."  More than once the idea has been floated that we should use view source as a "handle with care" solution, and it certainly does narrow the attack surface, but it doesn't eliminate it, despite creating that illusion.  Attacks against our network code, our header processing, our parsing, would all still work and so the appearance that it is "safe" to view source would be rather destructively misleading in many cases.

As you mention, bug 422410 was eventually resolved to allow users who need to see the page access.  We hope that users never do, but sysadmins may need to, and ultimately, as you can see in that bug, it was unclear which decision netted safer, in the end.  For the case you're describing, you could now visit the page and make the determination that way.  Yes, doing so would put you at risk (as would visiting it in another browser).  My advice if you wanted to investigate from a safe distance would be to use a tool like curl or wget to examine the page source without any interpretation, since even view source is no guarantee.

Comment 11

[Johnathan Nightingale [:johnath]]

(You'll note, though, that I haven't closed the bug as WONTFIX either - I understand that even after clicking through, view source won't behave itself - there is a legitimate bug here.  I guess I'm just disputing the notion that, because we allowed clickthrough, it therefore makes sense to also allow view-source.  Perhaps the right move is only to allow view-source post-clickthrough, though that will potentially be messy to implement.)

Comment 15

[Emil Ivanov]

As alternative you can use view-source: inside the urlbar like: view-source:http://www.mozilla.com/firefox/its-a-trap.html
and then click ignore this warning to see the page source.

Comment 20

[Clifton Royston]

Two further points:

1) As noted, this problem persists post-clickthrough.  The result is that the end-user is allowed to load the suspected malware page in the browser, with full exposure vulnerability, but not to view the source, which should surely expose only a subset of the vulnerabilities?

2) Google seems to be taking at least 2 weeks to remove sites from their "attack site" list, even after the cleanup is reported to them by a verified webmaster, and after they've recrawled the site.  The result is that even when the site is long-since fixed, it's not possible to use Firefox 3 to confirm that the suspect page is staying clean.  I particularly hate to use IE in this context, for obvious reasons, and don't think we should be forcing users in that direction.  Yes, I can go to a Unix box and use curl or wget but that probably makes me an atypical user.

FWIW, take it as the viewpoint of one who's currently going through the mill.

Comment 25

[Nathan]

I'd just like to clarify; nothing _actually_ works when you click view source, you can't "ignore the warning", you can't "get out of here" and you can't ask "why this site was blocked". It's buggy, not by design, it's just plain buggy.

Comment 26

[Nathan]

I'd also like to suggest a work-around  for those out there (which in itself could also be classified as a related bug). If you allow the page to display, and you'd like to see the source, simply "select all" and then "view selection source". Tada! No impassable "nag screen", yet arguably also not working by design, seeings as you're still "vulnerable" to whatever the DOM insepector is reading (which is effectively everything, including what's in <head />), you're just not warned about it.

Comment 27

[Nathan]

(In reply to comment #26)
> I'd also like to suggest a work-around  for those out there (which in itself
> could also be classified as a related bug). If you allow the page to display,
> and you'd like to see the source, simply "select all" and then "view selection
> source". Tada! No impassable "nag screen", yet arguably also not working by
> design, seeings as you're still "vulnerable" to whatever the DOM insepector is
> reading (which is effectively everything, including what's in <head />), you're
> just not warned about it.
> 

Sorry, calling it DOM Inspector is not really correct, but you know what I mean.

Comment 28

[Timwi]

Your workaround does not work. "View Selection Source" does not actually display the *source* (i.e. the HTML returned by the server). It displays Firefox's own reconstruction of the HTML from the DOM. This often no longer contains the contentious code that made the page a malware site in the first place.

Comment 29

[Dan Reif]

This is not a feature.  Disabling the "view source" option is perhaps a feature, though I'll contend it's a bit nanny-ish.  However, the current situation--want to view the source of the supposed attack page?  Ha!  Here, have some useless widgets to click on that do NOTHING!--is patently wrong.  Were it IE, I'd think that I *was* now infected merely by viewing the source; after all, my UI has stopped working!

Disallowing users to view source is a valid option, but presenting them with an entirely non-functional interface complete with entirely useless buttons and links is /not/.

Comment 30

[Nathan]

(In reply to comment #28)
> Your workaround does not work. "View Selection Source" does not actually
> display the *source* (i.e. the HTML returned by the server). It displays
> Firefox's own reconstruction of the HTML from the DOM. This often no longer
> contains the contentious code that made the page a malware site in the first
> place.
> 

Hi, sorry I intended for it to be a work-around if you needed to view source right away, using Firefox. I didn't intend for it to be a circumvention method as much as I meant for it to be a means of seeing some code while the "bug" gets sorted out (however it ends up going). My opinion on the matter is that you should be allowed to see the infected source code, I don't really mind if there is a warning, but you can see the actual page itself if you ignore the warning, so why not be allowed to see the - on most occasions - less harmful source code?

Comment 31

[Mike Connor [:mconnor]]

This isn't a blocker.  Its annoying, but its not something we can't ship with.

Comment 38

[Alan]

So other than just merge numerous bugs into one, is Mozille actually going to DO ANYTHING about this bug?

Comment 39

[Peter van der Woude [:Peter6]]

(In reply to comment #38)
> So other than just merge numerous bugs into one, is Mozille actually going to
> DO ANYTHING about this bug?
There's no need to yell, it does not help.
Obviously there are far more important bugs to fix.
Feel free to submit a full patch though.

Comment 41

[Daniel]

when is this going to be fixed? i suppose (i am not oriented in the firefox code) that this has to have a very easy fix (basically, just not showing the blocked screen on view source?)

Comment 45

[Johnathan Nightingale [:johnath]]

We wouldn't hold the release of Firefox 3.6 for this, the vast majority of our users are not viewing source on malware pages. A fix would be quite welcome, though the workaround in comment 15 makes me think more and more about how happy it would make me if view source moved into tabs (which would, implicitly, make this go away).

Comment 47

[L. Mohan Arun]

problem is that the buttons that get displayed in view source- warning window for malware pages are not working either..

Comment 48

[amster]

I never realised reporting this bug, and then it being marked as a duplicate of this bug, would then mean I would get a further 15 god damned F[expletive] emails regarding it.

I mean, if this is what happens for every bug one reports, then the mailbox will be under a deluge.

I'll never report a bug again - especially when a) it takes so long to fix b) when there's so many duplicates but yet nothing much is happening and/or c) when an approach to tackle the problem still hasn't yet been finalised!

How do I un-subscribe! Goodbye!

Comment 49

[Brendan Hide]

At the top right of the page at https://bugzilla.mozilla.org/show_bug.cgi?id=397937 there is a "CC list".

You can remove your email address from that list.

Comment 50

[Mauro V.]

Yes there any data for this bug to be corrected ?? is very anonyng I can't view the source of a page that as been listed as attacked.

I've to change to opera our Chrome to see the source...please correct this ASAP

Comment 51

[Dominic]

(In reply to comment #50)
> Yes there any data for this bug to be corrected ?? is very anonyng I can't view
> the source of a page that as been listed as attacked.
> 
> I've to change to opera our Chrome to see the source...please correct this ASAP

Here is a workaround: https://bugzilla.mozilla.org/show_bug.cgi?id=435726#c10

Comment 52

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1

This patch keeps the view-source functionality for blocked pages, and the warning pages (since view-source can potentially expose additional risk), but makes all the buttons on the warning page work.

Comment 53

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Forgot to mention: that patch also fixes bug 435726.

Comment 54

[Dão Gottwald [:dao]]

Comment on attachment 416840 [details] [diff] [review]
Patch v1

>+          let reportURL = formatter.formatURLPref("browser.safebrowsing.malware.reportURL", true);
>+          reportURL += errorDoc.location.href.slice(12);
>+          openURL(reportURL);

Two things... First, it looks like you need to use nsIURLFormatter befor using the pref value. Second, you're in toolkit, so reading a browser pref is probably not quite right.

Comment 55

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #54)
> Two things... First, it looks like you need to use nsIURLFormatter befor using
> the pref value. 

Er, huh?

> Second, you're in toolkit, so reading a browser pref is
> probably not quite right.

Ah snap - I missed that. That poses an interesting issue - any ideas?

Comment 56

[Dão Gottwald [:dao]]

(In reply to comment #55)
> Er, huh?

Alright, I somehow misread the formatter.formatURLPref part.

I don't have an idea for the second issue.

Comment 57

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

At least some of the browser.safebrowsing.* prefs are already effectively toolkit prefs (url-classifier code checks the .enabled prefs). I don't think we need to worry about using the infoURL pref in toolkit, really, as long as there's suitable fallback (might want to remove the reportError...).

Comment 58

[Reed Loden [:reed]]

(In reply to comment #57)
> At least some of the browser.safebrowsing.* prefs are already effectively
> toolkit prefs (url-classifier code checks the .enabled prefs). I don't think we
> need to worry about using the infoURL pref in toolkit, really, as long as
> there's suitable fallback (might want to remove the reportError...).

I would love to see any toolkit prefs living in browser/ moved to toolkit/. Linux distros are already having to do this with one url-classifier pref, so I'd love to see this fixed upstream on our end.

http://bazaar.launchpad.net/~mozillateam/xulrunner/xulrunner-1.9.3.head/annotate/head%3A/debian/patches/bzXXX_urlclassifier_prefs_in_toolkit.patch

Comment 59

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #57)
> At least some of the browser.safebrowsing.* prefs are already effectively
> toolkit prefs (url-classifier code checks the .enabled prefs). I don't think we
> need to worry about using the infoURL pref in toolkit

Alright - that makes me feel better. The urlclassifier/safebrowsing split is a bit awkward at times.

> as long as there's suitable fallback (might want to remove the reportError...).

FWIW, reportError() is what browser code uses for that case. Any fallback should apply to both cases. Ideally, I'd like the buttons that open these URLs to just not show if the relevant pref is missing (or malformed). The text on these warning pages don't reference the buttons, so the pages will still make sense if the buttons are hidden. Filed bug 534545 for this.

Comment 60

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #58)
> I would love to see any toolkit prefs living in browser/ moved to toolkit/.
> Linux distros are already having to do this with one url-classifier pref, so
> I'd love to see this fixed upstream on our end.

Was just thinking that too - do we have a bug for that?

Comment 61

[Reed Loden [:reed]]

(In reply to comment #60)
> (In reply to comment #58)
> > I would love to see any toolkit prefs living in browser/ moved to toolkit/.
> > Linux distros are already having to do this with one url-classifier pref, so
> > I'd love to see this fixed upstream on our end.
> 
> Was just thinking that too - do we have a bug for that?

Not that I know of... could you file one, or would you like me to file it?

Comment 62

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

(In reply to comment #59)
> > as long as there's suitable fallback (might want to remove the reportError...).
> 
> FWIW, reportError() is what browser code uses for that case.

Right, but that pref is set in browser's default prefs. My point was that in the case of a generic toolkit app that doesn't set the pref, we might actually want to treat this like an ignorable failure. I didn't think through what that would mean, though (user clicks a button and nothing happens), so just ignore that comment - the reportError() is fine.

Comment 63

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

I filed bug 534580 about auditing toolkit prefs.

Comment 65

[Mike Connor [:mconnor]]

Comment on attachment 416840 [details] [diff] [review]
Patch v1

>diff --git a/browser/components/safebrowsing/content/blockedSite.xhtml b/browser/components/safebrowsing/content/blockedSite.xhtml
>       function getURL()
>       {
>         var url = document.documentURI;
>-        var index = url.search(/u\=/);
>+        var match = url.match(/&u=([^&]+)&/);
> 
>-        // index == -1 if not found; if so, return an empty string
>+        // match == null if not found; if so, return an empty string
>         // instead of what would turn out to be portions of the URI
>-        if (index == -1)
>+        if (!match)
>           return "";
> 
>-        return decodeURIComponent(url.slice(index + 2));
>+        var url = decodeURIComponent(match[1]);

no need to redeclare url.  Though, I'd make this into an nsIURI and use schemeIs, rather than rely on decodeURIComponent to be what we want.

>diff --git a/toolkit/components/viewsource/content/viewSource.js b/toolkit/components/viewsource/content/viewSource.js

>+function onCommandContent(event) {
>+  // Don't trust synthetic events
>+  if (!event.isTrusted)
>+    return;
>+
>+  var ot = event.originalTarget;

s/ot/target/ for readability in context.

>+  if (/^about:blocked/.test(errorDoc.documentURI)) {
>+    // The event came from a button on a malware/phishing block page
>+    // First check whether it's malware or phishing, so that we can
>+    // use the right strings/links
>+    var isMalware = /e=malwareBlocked/.test(errorDoc.documentURI);
>+    
>+    if (ot == errorDoc.getElementById('getMeOutButton')) {
>+      // Instead of loading some safe page, just close the window
>+      window.close();
>+    } else if (ot == errorDoc.getElementById('reportButton')) {
>+      // This is the "Why is this site blocked" button.  For malware,
>+      // we can fetch a site-specific report, for phishing, we redirect
>+      // to the generic page describing phishing protection.
>+
>+      if (isMalware) {
>+        // Get the stop badware "why is this blocked" report url,
>+        // append the current url, and go there.
>+        try {
>+          let reportURL = formatter.formatURLPref("browser.safebrowsing.malware.reportURL", true);
>+          reportURL += errorDoc.location.href.slice(12);
>+          openURL(reportURL);
>+        } catch (e) {
>+          Components.utils.reportError("Couldn't get malware report URL: " + e);
>+        }
>+      } else { // It's a phishing site, not malware
>+        try {
>+          var infoURL = formatter.formatURLPref("browser.safebrowsing.warning.infoURL", true);
>+          openURL(infoURL);
>+        } catch (e) {
>+          Components.utils.reportError("Couldn't get phishing info URL: " + e);
>+        }
>+      }
>+    } else if (ot == errorDoc.getElementById('ignoreWarningButton')) {
>+      // Allow users to override and continue through to the site,
>+      // but add a notify bar as a reminder, so that they don't lose
>+      // track after, e.g., tab switching.
>+      gBrowser.loadURIWithFlags(content.location.href,
>+                                Ci.nsIWebNavigation.LOAD_FLAGS_BYPASS_CLASSIFIER,
>+                                null, null, null);

style bitching: I know that this file uses the "} else {" style more often, but we've been trying to get away from that, so please don't tilt the balance that way. :)

I'm not super worried about the prefs, since they're needed for SB, so any consumers will probably have them.. need to clean up naming though.

Comment 66

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

(In reply to comment #65)
> Though, I'd make this into an nsIURI and use schemeIs, rather than rely on
> decodeURIComponent to be what we want.

Can't, since the page is unprivileged.

> I know that this file uses the "} else {" style more often, but
> we've been trying to get away from that

Death to }\nelse{!!!!!!!!!!!!!!!

Comment 67

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #66)
> > I know that this file uses the "} else {" style more often, but
> > we've been trying to get away from that
> 
> Death to }\nelse{!!!!!!!!!!!!!!!

Seconded!

Comment 68

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1.1

Tiny changes recommended in comment 65. Ready to land.

Comment 69

[Nochum Sossonko [:Natch]]

Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/454c8ec86b1e.

Shouldn't this get a test?

Comment 70

[u279076]

I'm a little confused as to what the desired behaviour is here...

1. Go to malware site
2. Warning page is displayed
3. View Page source

RESULT:
Warning page displayed in View Source window.
Cannot click buttons or Ignore link in the View Source window
Clicking buttons or Ignore link in the main window loads the correct pages in the main window

EXPECTED:
???

Comment 71

[u279076]

Sorry, I should add that I am using the following build:
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2) Gecko/20100105 Firefox/3.6

Comment 72

[Mike Connor [:mconnor]]

It's not in 3.6...

Comment 73

[u279076]

(In reply to comment #72)
> It's not in 3.6...

Ok, so I'll check minefield.  This still does not answer my question of what expected behaviour to "expect"...

Comment 74

[Timwi]

Personally, my expectation is that if I select "View page source", it displays the page source. I'm somewhat intrigued by the fact that you didn't think of that?

Comment 75

[u279076]

(In reply to comment #74)
> Personally, my expectation is that if I select "View page source", it displays
> the page source. I'm somewhat intrigued by the fact that you didn't think of
> that?

Well, I did think of that actually.  I was really just asking what to expect the behaviour to be given the code changes that were made with the patch.  I'm not really interested in getting into a UX debate.

Comment 76

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment 52 explains the expected behavior. 

It can't be guaranteed that view-source isn't exploitable (network calls, HTTP, parsing, formatting, linkification, etc etc). As such, view-source on a blocked page WILL still show the safebrowsing warning. What this patch does is make the buttons on the warning page work, so you can bypass that warning.

Comment 77

[u279076]

(In reply to comment #76)
> Comment 52 explains the expected behavior. 
> 
> It can't be guaranteed that view-source isn't exploitable (network calls, HTTP,
> parsing, formatting, linkification, etc etc). As such, view-source on a blocked
> page WILL still show the safebrowsing warning. What this patch does is make the
> buttons on the warning page work, so you can bypass that warning.

That you for taking your time to explain.  This is very much appreciated.

Comment 78

[u279076]

*Thank you, not "That you" :P

Comment 79

[u279076]

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20100112 Minefield/3.7a1pre

Works as expected:
View Page Source -> Warning page
Ignore this Warning -> Shows the source
Get me outta here -> Closes the VPS window
Why was this page blocked -> Opens the Google safebrowsing page in a new tab of the main window

Marking verified.

Comment 80

[u279076]

Litmus test case added: https://litmus.mozilla.org/show_test.cgi?id=9881

Comment 83

[p.nospam.astfeld]

Sorry for my mistake(not finding duplicates)
What do you think about $simple_editor(vi, notepad,...) $Sourcecode?

Comment 85

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #83)
> What do you think about $simple_editor(vi, notepad,...) $Sourcecode?

You mean opening the source in an external editor? You should file a new bug for that (specifically regarding use of an external editor); however it sounds like that functionality should be provided by an extension, not built-in.

Comment 86

[Johnathan Nightingale [:johnath]]

(In reply to comment #85)
> (In reply to comment #83)
> > What do you think about $simple_editor(vi, notepad,...) $Sourcecode?
> 
> You mean opening the source in an external editor? You should file a new bug
> for that (specifically regarding use of an external editor); however it sounds
> like that functionality should be provided by an extension, not built-in.

And, in fact, is. ViewSourceWith does this, though it's a bit behind the compat curve last I checked. The Web Developer Toolbar also lets you set up alternate source viewers in their preferences dialog, and even lets you bind different external tools to different hotkeys.

Comment 87

[p.nospam.astfeld]

Oh, nice.
view_source.editor.path + view_source.editor.external is perfekt.
But not with the Testcase ID #9881 (http://www.mozilla.com/firefox/its-an-attack.html) (I use FF 3.6)

What Do you think about to remove this security feature when using an external editor?
BTW:
Thanks for your work ;-)

Comment 88

[Andy Powell]

"It can't be guaranteed that view-source isn't exploitable (network calls, HTTP,
parsing, formatting, linkification, etc etc)."

I guess the next question is why is view source doing anything at all, surely it shouldn't be formatting or parsing anything it should just be a raw dump of the html not some beautified representation of what the html might have looked like in an ideal world.

Comment 89

[Johnathan Nightingale [:johnath]]

(In reply to comment #88)
> I guess the next question is why is view source doing anything at all, surely
> it shouldn't be formatting or parsing anything it should just be a raw dump of
> the html not some beautified representation of what the html might have looked
> like in an ideal world.

Your "surely" there goes too far. We could secure the entire avenue of attack by just removing view source altogether, but that would be over-restrictive because we believe that view source provides a function which is valuable to our users and aligned with our mission. Indeed, the ability to view the source of a web page is central to its openness. For similar reasons, we hold ourselves to a higher standard in terms of the quality of that experience than "straight dump of bytes" - you can have that with wireshark if you want, and skip the "ignore this warning" step completely. Our view source is linkified, syntax highlighted, and supports text- and position-searching because those make it a more effective tool. People who click through the (double!) warnings to view the source are, implicitly, signing up for a marginal increase in risk by doing so. We're not going to remove the functionality in order to mitigate that, though.

Comment 90

[Andy Powell]

(In reply to comment #89)
> 
> Your "surely" there goes too far. We could secure the entire avenue of attack
> by just removing view source altogether, but that would be over-restrictive

I disagree. You could remove the entire avenue of attack by simply not parsing at all when using view source. There are excellent plug-ins like firebug or view source chart that provide this functionality.


> because we believe that view source provides a function which is valuable to
> our users and aligned with our mission. Indeed, the ability to view the source
> of a web page is central to its openness. For similar reasons, we hold


You say that "ability to view the source of a web page is central to its openness" yet I can't view the source of the page, I have to view a *modified* version of it, a version modified for pretty display.




> ourselves to a higher standard in terms of the quality of that experience than
> "straight dump of bytes" - you can have that with wireshark if you want


Sure I can get that with wireshark, wget, I can even get it with IE 4 - that doesn't make it an ideal solution. The majority of people who are going to be looking at the source of a page are likely to be just the sort of people who don;t need to see  beautified/modified source. It makes for very difficult debugging. 


> skip the "ignore this warning" step completely. Our view source is linkified,
> syntax highlighted, and supports text- and position-searching because those
> make it a more effective tool. People who click through the (double!) warnings
> to view the source are, implicitly, signing up for a marginal increase in risk
> by doing so. We're not going to remove the functionality in order to mitigate
> that, though.

The only reason clicking through poses any risk at all is because the raw source isn't what is being shown, it's being parsed.

Personally I wouldn't see it as 'removing functionality' I'd see it as restoring proper functionality.

Comment 91

[Mauro V.]

(In reply to comment #90)
> (In reply to comment #89)
> > 
> > Your "surely" there goes too far. We could secure the entire avenue of attack
> > by just removing view source altogether, but that would be over-restrictive
> 
> I disagree. You could remove the entire avenue of attack by simply not parsing
> at all when using view source. There are excellent plug-ins like firebug or
> view source chart that provide this functionality.
> 


Amen, I subscribe everything you just said.

I don't want to change to another browser only to see the source code of a web page, because of a dumb version of shouw sorce from firefox.

I don't want a prety show source, i wan't to see the SOURCE of the web page and find the problem or anything about the webpage nothing more.


> 
> > because we believe that view source provides a function which is valuable to
> > our users and aligned with our mission. Indeed, the ability to view the source
> > of a web page is central to its openness. For similar reasons, we hold
> 
> 
> You say that "ability to view the source of a web page is central to its
> openness" yet I can't view the source of the page, I have to view a *modified*
> version of it, a version modified for pretty display.
> 
> 
> 
> 
> > ourselves to a higher standard in terms of the quality of that experience than
> > "straight dump of bytes" - you can have that with wireshark if you want
> 
> 
> Sure I can get that with wireshark, wget, I can even get it with IE 4 - that
> doesn't make it an ideal solution. The majority of people who are going to be
> looking at the source of a page are likely to be just the sort of people who
> don;t need to see  beautified/modified source. It makes for very difficult
> debugging. 
> 
> 
> > skip the "ignore this warning" step completely. Our view source is linkified,
> > syntax highlighted, and supports text- and position-searching because those
> > make it a more effective tool. People who click through the (double!) warnings
> > to view the source are, implicitly, signing up for a marginal increase in risk
> > by doing so. We're not going to remove the functionality in order to mitigate
> > that, though.
> 
> The only reason clicking through poses any risk at all is because the raw
> source isn't what is being shown, it's being parsed.
> 
> Personally I wouldn't see it as 'removing functionality' I'd see it as
> restoring proper functionality.

Comment 98

[Jez]

(In reply to comment #79)
> Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20100112
> Minefield/3.7a1pre
> 
> Works as expected:
> View Page Source -> Warning page
> Ignore this Warning -> Shows the source
> Get me outta here -> Closes the VPS window
> Why was this page blocked -> Opens the Google safebrowsing page in a new tab of
> the main window
> 
> Marking verified.

This works fine for the built-in source viewer, but I have a custom one defined via the view_source.editor.path pref.  Will it fix the source not being sent to the custom source viewer, too?

Comment 99

[Radek 'sysKin' Czyz]

(In reply to comment #69)
> Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/454c8ec86b1e.
> 
> Shouldn't this get a test?

Did it ever get a test? Because this bug is present in 4.0 as well as today's Nightly, so I suppose it regressed somewhere along the way.

Comment 100

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

(In reply to comment #99)
> this bug is present in 4.0 as well as today's Nightly, so I suppose it
> regressed somewhere along the way.

I filed bug 652542.