Most of the discussion is in bug 325058 already, but here is a short summary: bug 325058 comment 0 (LpSolit): This URL ( http://landfill.bugzilla.org//paul/ ) contains a double slash // and the login form on Bugzilla redirects me outside bugzilla.org, in my case paul.fr (but I suspect it depends on your locale, in my case: french). Discussing with CTho on #developers, it looks like the login and password [may] have been sent to paul.fr instead of landfill.bugzilla.org, meaning that they have now my admin login and password for our QA installations. bug 325058 comment 6 (dveditz): Is this a landfill bug in the way it handles multiple installations? The form action is derived from the URL. If you don't specify the double-slash and visit the intended http://landfill.bugzilla.org/paul/ then the form action is "/paul/index.cgi" which logs into landfill. If you use the double-slash it's copied into the form action making 'paul' a hostname. Landfill should do some sanity-checking when generating the form action, or specify the complete url. Why is this filed as a networking bug? It looks like Gecko is doing exactly what the page is telling it to do (submit the form to "paul"). bug 325058 comment 8 (LpSolit): The problem is not specific to landfill, it affects *all* Bugzilla installations. The problem is that template/en/default/account/auth/login-small.html.tmpl uses: <form name="login" action="[% cgi.script_name FILTER html %]" method="POST"> so when you enter //foo, you get action="//foo/index.cgi" which redirects you outside your Bugzilla installation with your login and password. It could as well be a bug in CGI.pm itself. Anyway, Bugzilla should probably use Param('urlbase'). ----- The template/en/default/account/auth/login-small.html.tmpl template was introduced in Bugzilla 2.20 in bug 165075. This means that only the home page is affected due to the use of 'cgi.script_name' which returns the full path, but you are not affected when you have to log in due to GoAheadAndLogIn=1 which is correctly handled by Auth::Login::WWW::CGI::login(). This routine indeed calls $cgi->url(-relative=>1) and so you cannot be redirected outside Param('urlbase') or Param('sslbase'). Probably the right fix is to do something similar for login-small.html.tmpl.
I don't know if we want to block 2.20.1 for it.
It should be noted that this is only an issue if the "foo" part of "//foo" is a valid URL - in the example given, no data is sent unless the address "paul" resolves to something. So the bugzilla install would have to be at an address like "www.domain.org/evil.com" for this to be exploitable, and it would be exploitable only by evil.com.
// is shorthand for a Windows network share, isn't it? So this would look for an smb:// host on your local network named "paul" instead of http: if it's interpreting it as a hostname.
*** Bug 325058 has been marked as a duplicate of this bug. ***
I don't particularly see this as major. A user has to type a double slash, or be redirected to the site with a double slash. Now, granted, somebody could maliciously link to the site with a double slash, so that's exploitable. But for the most part, this would only be exploited by accident. After all, this bug has obviously been around for a long time, and we only just now discovered it. I'm hesitant to block anything on it, but as it is a valid security bug, we should fix it before our *next* (not 2.20.1) release.
Oh, by the way, I think the obvious simple fix is to modify the URL filter so that it strips off any slashes near the beginning of a URL other than the first one. It means that we won't be able to insert SMB network paths there, but... um... oh well. :-) I don't think we had any plans on that.
(In reply to comment #6) > Oh, by the way, I think the obvious simple fix is to modify the URL filter so > that it strips off any slashes near the beginning of a URL other than the first > one. It means that we won't be able to insert SMB network paths there, but... > um... oh well. :-) I don't think we had any plans on that. > No need to change the URL filter. The right fix is to replace cgi.script_name by something like $cgi->url(-relative => 1), as done by Auth::Login::WWW::CGI::login().
(In reply to comment #5) > for the most part, this would only be exploited by accident. After all, this > bug has obviously been around for a long time, and we only just now discovered > it. "for a long time" isn't true. Only versions 2.19.3 and above are affected, with 2.20 being released on Sept 30, 2005; that's pretty recent. 2.19.2 and below are not affected as you couldn't log in from the home page.
Created attachment 211032 [details] [diff] [review] possible fix, v1 cgi.script_name will redirect you outside your installation if the URL contains //. So I now let the .cgi script pass this param.
Created attachment 211090 [details] [diff] [review] better patch, v2 No need to hack index.cgi anymore.
Comment on attachment 211090 [details] [diff] [review] better patch, v2 >+[% DEFAULT homepage = "index.cgi" %] Nit: this should probably be set globally, and perhaps it should be an absolute URL via URLPATH.
Created attachment 211095 [details] [diff] [review] patch, v3 Use an absolute path. Now, //paul and //paul/index.cgi both fall back to /paul/index.cgi.
Comment on attachment 211095 [details] [diff] [review] patch, v3 >Index: template/en/default/account/auth/login-small.html.tmpl >=================================================================== >RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/account/auth/login-small.html.tmpl,v >retrieving revision 1.1 >diff -u -r1.1 login-small.html.tmpl >--- template/en/default/account/auth/login-small.html.tmpl 12 Apr 2005 17:23:01 -0000 1.1 >+++ template/en/default/account/auth/login-small.html.tmpl 8 Feb 2006 02:21:24 -0000 >@@ -21,7 +21,20 @@ > > [% PROCESS global/variables.none.tmpl %] > >-<form name="login" action="[% cgi.script_name FILTER html %]" method="POST"> >+[%# Use the current script name. If an empty name is retuned, >+ # then we are accessing the home page. %] Nit: retuned -> returned >+[% DEFAULT homepage = "index.cgi" %] >+[% script_name = cgi.url(Relative => 1) || homepage %] >+ >+[%# If SSL is in use, use 'sslbase', else use 'urlbase'. %] >+[% IF Param("sslbase") != "" && Param("ssl") != "never" %] >+ [% script_name = Param("sslbase") _ script_name %] >+[% ELSE %] >+ [% script_name = Param("urlbase") _ script_name %] >+[% END %] I wouldn't think this particularly necessary unless cgi.url is empty, but I doubt it hurts anything, either. It does seem like this stuff should happen globally, so all templates can take advantage of it.
(In reply to comment #13) > I wouldn't think this particularly necessary unless cgi.url is empty, but I > doubt it hurts anything, either. It does seem like this stuff should happen > globally, so all templates can take advantage of it. > Well, I could still write http://www.foo.com//paul/index.cgi. My patch force it to fall back to http://www.foo.com/paul/index.cgi in this case too. About making it global, we could set <base href=""> in global/header.html.tmpl, but I'm not sure <form> really cares about <base>. Does it?
Created attachment 211115 [details] [diff] [review] patch, v3.1 Small improvement: do not hardcode 'index.cgi'. If we are accessing the homepage, the relative URL is empty, but as we are appending the full path to the installation (either urlbase or sslbase), we never end with an empty script_name and so we never have action="".
tip: Checking in template/en/default/account/auth/login-small.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/account/auth/login-small.html.tmpl,v <-- login-small.html.tmpl new revision: 1.2; previous revision: 1.1 done 2.20: Checking in template/en/default/account/auth/login-small.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/account/auth/login-small.html.tmpl,v <-- login-small.html.tmpl new revision: 22.214.171.124; previous revision: 1.1 done
Security Advisory posted; unlocking bug.
Blocker regression caused by this bug at bug 328108 (maybe we should consider some of the previous versions, in order to avoid using urlbase in a pre-login stage)