SecurityError in Javascript on buglist.cgi

RESOLVED FIXED in Bugzilla 4.4

Status

()

P1
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mail, Assigned: mail)

Tracking

({regression})

Bugzilla 4.4
regression
Bug Flags:
approval +
approval4.4 +
blocking4.4.1 -

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Created attachment 753128 [details] [diff] [review]
v1 patch

Brc have web servers that run behind proxies. The communication between the proxy and the web server is done insecurely, on port 80. Because of this, CGI->self_url presents the URL as http://bugzilla.redhat.com/....

On a modern browser, buglist.cgi does a post and history.replaceState does it magic. The problem is that the redirect is to http://bugzilla.redhat.com/buglist.cgi?....

This leads to an Javascript error "SecurityError: The operation is insecure"

This patch fixes that so it always generates a relative URL.

There is a call to CGI->self_url in Bugzilla::CGI, but that is okay to leave as is.
Attachment #753128 - Flags: review?(LpSolit)
Flags: blocking4.4.1?

Comment 1

5 years ago
How is that a bug? Your configuration should have ssl_redirect = on if you want HTTPS. Either that, or I don't understand your problem correctly.

Moreover, I don't understand where you see the problem. Please provide detailed STR.
(Assignee)

Comment 2

5 years ago
This is the configuration we have.

Our two web servers (that have the bugzilla code) run on port 80, and do http. ssl_redirect is 0 (since these servers don't do SSL), and the urlbase and sslbase are both https://bugzilla.redhat.com/

In front of these webservers we have a proxy server (which is where bugzilla.redhat.com IP address points to), that listens on port 443 (https). These forward the requests on to the two webservers via http.

After speaking to glob, I believe that bmo have the same configuration. I have confirmed that urlbase on bmo is https://bugzilla.mozilla.org/ too, and the communication between the proxy and the Bugzilla web server is done over http.

Because CGI->self_url looks at the port when generating a URL, the offending code makes the URL http://bugzilla.redhat.com/buglist.cgi?... and when used in a history.replaceState the browser rejects this, because the link is http when the site is https.

It probably only affects bmo and brc, but it is still something that I think should be fixed upstream.

My patch removes the use of CGI->self_url.

  -- simon
(Assignee)

Comment 3

5 years ago
Created attachment 754315 [details] [diff] [review]
v2 patch

My first patch broke saved searches. This should fix that problem.
Attachment #753128 - Attachment is obsolete: true
Attachment #753128 - Flags: review?(LpSolit)
Attachment #754315 - Flags: review?(LpSolit)

Comment 4

5 years ago
I don't see how this is a critical bug. All it does is that the error is thrown in the Error Console (which you don't see by default), and the URL doesn't contain the query string. But you can still edit the query, and you can still save it. This has no impact on the UI nor on the usability. Also, it's limited to the use of a proxy using the same configuration as you, which shouldn't impact too many installations.

I won't block 4.4.1 on it. But the fix can still go in when it's reviewed, of course. Tell me if I miss a use case which makes this problem more severe.
Severity: critical → normal
Depends on: 730670
Flags: blocking4.4.1? → blocking4.4.1-
Keywords: regression

Comment 5

5 years ago
Comment on attachment 754315 [details] [diff] [review]
v2 patch

>-  [% new_url = cgi.self_url %]
>+  [% new_url = 'buglist.cgi?' _ cgi.canonicalise_query %]

While this would work, you should avoid calling canonicalise_query() twice. I much prefer something like this:

  [% new_url = 'buglist.cgi?' %]
  [% IF quicksearch %]
    ...
  [% ELSIF cgi.param('token') != '' %]
    ...
  [% ELSE %]
    ...
  [% END %]

As new_url always begins with "buglist.cgi?", you would simply have to append the right query string to it.
Attachment #754315 - Flags: review?(LpSolit) → review-
(Assignee)

Comment 6

5 years ago
Created attachment 758900 [details] [diff] [review]
v3 patch
Attachment #754315 - Attachment is obsolete: true
Attachment #758900 - Flags: review?(LpSolit)

Updated

5 years ago
Attachment #758900 - Flags: review?(LpSolit) → review?(glob)
Comment on attachment 758900 [details] [diff] [review]
v3 patch

r=glob
Attachment #758900 - Flags: review?(glob) → review+
Flags: approval4.4+
Flags: approval+
(Assignee)

Comment 8

5 years ago
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified template/en/default/list/list.html.tmpl
Committed revision 8729.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.4/
modified template/en/default/list/list.html.tmpl
Committed revision 8603.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.