Closed
Bug 325079
Opened 18 years ago
Closed 18 years ago
[SECURITY] The login form on the Bugzilla home page may redirect your login and password to another site
Categories
(Bugzilla :: User Accounts, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.20
People
(Reporter: LpSolit, Assigned: LpSolit)
References
()
Details
(Whiteboard: [doesn't affect 2.16][doesn't affect 2.18][ready for 2.20.1][ready for 2.22rc1])
Attachments
(1 file, 3 obsolete files)
1.13 KB,
patch
|
myk
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•18 years ago
|
||
I don't know if we want to block 2.20.1 for it.
Flags: blocking2.22?
Flags: blocking2.20.2?
Flags: blocking2.20.1?
Comment 2•18 years ago
|
||
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.
Comment 3•18 years ago
|
||
// 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.
Comment 4•18 years ago
|
||
*** Bug 325058 has been marked as a duplicate of this bug. ***
Comment 5•18 years ago
|
||
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.
Severity: major → normal
Flags: blocking2.22?
Flags: blocking2.22+
Flags: blocking2.20.2?
Flags: blocking2.20.2+
Flags: blocking2.20.1?
Flags: blocking2.20.1-
Comment 6•18 years ago
|
||
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.
Assignee | ||
Comment 7•18 years ago
|
||
(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().
Assignee | ||
Comment 8•18 years ago
|
||
(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.
Assignee | ||
Comment 9•18 years ago
|
||
cgi.script_name will redirect you outside your installation if the URL contains //. So I now let the .cgi script pass this param.
Attachment #211032 -
Flags: review?(justdave)
Assignee | ||
Comment 10•18 years ago
|
||
No need to hack index.cgi anymore.
Assignee: user-accounts → LpSolit
Attachment #211032 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #211090 -
Flags: review?(myk)
Attachment #211032 -
Flags: review?(justdave)
Comment 11•18 years ago
|
||
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.
Attachment #211090 -
Flags: review?(myk) → review+
Assignee | ||
Comment 12•18 years ago
|
||
Use an absolute path. Now, //paul and //paul/index.cgi both fall back to /paul/index.cgi.
Attachment #211090 -
Attachment is obsolete: true
Attachment #211095 -
Flags: review?(myk)
Comment 13•18 years ago
|
||
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.
Attachment #211095 -
Flags: review?(myk) → review+
Assignee | ||
Comment 14•18 years ago
|
||
(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?
Assignee | ||
Updated•18 years ago
|
Whiteboard: [ready for 2.20.1][ready for 2.22rc1]
Assignee | ||
Comment 15•18 years ago
|
||
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="".
Attachment #211095 -
Attachment is obsolete: true
Attachment #211115 -
Flags: review?(myk)
Updated•18 years ago
|
Attachment #211115 -
Flags: review?(myk) → review+
Assignee | ||
Updated•18 years ago
|
Whiteboard: [ready for 2.20.1][ready for 2.22rc1] → [doesn't affect 2.16][doesn't affect 2.18][ready for 2.20.1][ready for 2.22rc1]
Assignee | ||
Updated•18 years ago
|
Version: 2.20 → 2.19.3
Updated•18 years ago
|
Flags: approval?
Flags: approval2.20?
Flags: approval2.20+
Flags: approval+
Assignee | ||
Comment 16•18 years ago
|
||
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: 1.1.4.1; previous revision: 1.1 done
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Comment 18•18 years ago
|
||
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)
Assignee | ||
Updated•18 years ago
|
Summary: The login form on the Bugzilla home page may redirect your login and password to another site → [SECURITY] The login form on the Bugzilla home page may redirect your login and password to another site
You need to log in
before you can comment on or make changes to this bug.
Description
•