Viewing source of "Reported Attack Site"

see also bug 273968

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.

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

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.

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.

Created an attachment (id=289366)
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

*** Bug 425550 has been marked as a duplicate of this bug. ***

*** Bug 396308 has been marked as a duplicate of this bug. ***

@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.

(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.

(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.)

*** Bug 431026 has been marked as a duplicate of this bug. ***

*** Bug 431099 has been marked as a duplicate of this bug. ***

*** Bug 431230 has been marked as a duplicate of this bug. ***

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.

*** Bug 434288 has been marked as a duplicate of this bug. ***

Steps to reproduce:

1) Load page http://www.mozilla.com/firefox/its-an-attack.html
2) Click Ignore link so you can see "It’s an Attack!" page.
3) Now press CTRL+U to see its HTML source code

Results:
HTML souce code is inaccessible. In the source code window is Antiphishing warning again and you cannot pass through it.

Confirming with
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008052506 Minefield/3.0pre

*** Bug 435726 has been marked as a duplicate of this bug. ***

*** This bug has been marked as a duplicate of bug 397937 ***

This isn't quite the same bug as bug 397937, given step 2), but it seems likely that any solution to it would depend on the a fix for bug 397937.

I agree, bug 397937 is not duplicate of this bug. In fact, bug 397937 is not actually a bug. But this one definitely is. If I agree to view malicious page as is, why can't I view its source?

*** Bug 436536 has been marked as a duplicate of this bug. ***

*** Bug 437976 has been marked as a duplicate of this bug. ***

*** Bug 439175 has been marked as a duplicate of this bug. ***

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.

*** Bug 439744 has been marked as a duplicate of this bug. ***

*** Bug 442661 has been marked as a duplicate of this bug. ***

*** Bug 443867 has been marked as a duplicate of this bug. ***

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.

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.

(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.

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.

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/.

I filed bug 534580 about auditing toolkit prefs.

*** Bug 534594 has been marked as a duplicate of this bug. ***

(From update of attachment 416840)
>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...

(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{!!!!!!!!!!!!!!!

(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!

Created an attachment (id=418586)
Patch v1.1

Tiny changes recommended in comment 65. Ready to land.

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

Shouldn't this get a test?

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:
???

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

It's not in 3.6...

(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"...

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?

(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 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.

(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.

*Thank you, not "That you" :P

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 was fixed with bug 397937.
http://hg.mozilla.org/mozilla-central/rev/454c8ec86b1e

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

*** Bug 548568 has been marked as a duplicate of this bug. ***

*** Bug 550075 has been marked as a duplicate of this bug. ***

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

*** Bug 550215 has been marked as a duplicate of this bug. ***

(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.

(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.

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 ;-)

"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.

(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.

(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.

(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.

*** Bug 562117 has been marked as a duplicate of this bug. ***

*** Bug 570273 has been marked as a duplicate of this bug. ***

*** Bug 573208 has been marked as a duplicate of this bug. ***

*** Bug 573916 has been marked as a duplicate of this bug. ***

*** Bug 578304 has been marked as a duplicate of this bug. ***

*** Bug 600126 has been marked as a duplicate of this bug. ***

(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?

(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.

(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.