Closed Bug 875217 Opened 9 years ago Closed 8 years ago

SecurityError in Javascript on buglist.cgi


(Bugzilla :: Query/Bug List, defect, P1)




Bugzilla 4.4


(Reporter: mail, Assigned: mail)



(Keywords: regression)


(1 file, 2 obsolete files)

Attached patch v1 patch (obsolete) — Splinter Review
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

On a modern browser, buglist.cgi does a post and history.replaceState does it magic. The problem is that the redirect is to

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

In front of these webservers we have a proxy server (which is where 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 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 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
Attached patch v2 patch (obsolete) — Splinter Review
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)
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 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-
Attached patch v3 patchSplinter Review
Attachment #754315 - Attachment is obsolete: true
Attachment #758900 - Flags: review?(LpSolit)
Attachment #758900 - Flags: review?(LpSolit) → review?(glob)
Comment on attachment 758900 [details] [diff] [review]
v3 patch

Attachment #758900 - Flags: review?(glob) → review+
Flags: approval4.4+
Flags: approval+
Committing to: bzr+ssh://
modified template/en/default/list/list.html.tmpl
Committed revision 8729.

Committing to: bzr+ssh://
modified template/en/default/list/list.html.tmpl
Committed revision 8603.
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.