Description
Dobrica Pavlinusic
2015-03-04 08:55:25 UTC
Created attachment 36445 [details] [review] Bug 13789 - facets with accented utf-8 characters generate double encoded links Bug 13425 tried to fix XSS in OPAC, but as result it double encoded strings in facets and made links with accented characters (which where encoded by url filter) broken. This is just a test commit to see wheater removing url filter fixes this (it does), but it also re-introduces XSS bug so it's not ready for signoff. Created attachment 36477 [details] [review] Bug 13789 - facets with accented utf-8 characters generate double encoded links Bug 13425 tried to fix XSS in OPAC, by using url filter in template toolkit on whole generated url. This doesn't work and create double encoded strings in facets because we are creating url variable by concatenating query_cgi (which did pass through uri_escape_utf8 on perl side) and other parameters which have to be escaped in template. Also, code like [% SET limit_cgi_f = limit_cgi | url %] doesn't do anything (at least doesn't apply url filter) so it's not needed. Test scenario: 1. find results in your opac which contain accented characters 2. click on them and verify that results are missing 3. apply this patch 4. re-run search and click on facets link verifying that there are now results 5. verify that facets are still safe from injection by constructing url like /cgi-bin/koha/opac-search.pl?q=123&sort_by='"><script>prompt('Happy_Holidays')</script>&limit=123 and verifying that you DON'T see prompt window in your browser Sorry Dobrica, I found an issue :-/ The encoding problem is fixed for the facet lib, but try to click on one facet, it should be selected (i.e. not a link and with the [x] to remove it), and it's not. It looks like there is another encoding issue somewhere else... Created attachment 36501 [details] [review] Bug 13789 - facets with accented utf-8 characters generate double encoded links Bug 13425 tried to fix XSS in OPAC, by using url filter in template toolkit on whole generated url. This doesn't work and create double encoded strings in facets because we are creating url variable by concatenating query_cgi (which did pass through uri_escape_utf8 on perl side) and other parameters which have to be escaped in template. Also, code like [% SET limit_cgi_f = limit_cgi | url %] doesn't do anything (at least doesn't apply url filter) so it's not needed. Test scenario: 1. find results in your opac which contain accented characters 2. click on them and verify that results are missing 3. apply this patch 4. re-run search and click on facets link verifying that there are now results 5. verify that facets are still safe from injection by constructing url like /cgi-bin/koha/opac-search.pl?q=123&sort_by='"><script>prompt('Happy_Holidays')</script>&limit=123 and verifying that you DON'T see prompt window in your browser Created attachment 36540 [details] [review] Bug 13789 - facets with accented utf-8 characters generate double encoded links Bug 13425 tried to fix XSS in OPAC, by using url filter in template toolkit on whole generated url. This doesn't work and create double encoded strings in facets because we are creating url variable by concatenating query_cgi (which did pass through uri_escape_utf8 on perl side) and other parameters which have to be escaped in template. Also, code like [% SET limit_cgi_f = limit_cgi | url %] doesn't do anything (at least doesn't apply url filter) so it's not needed. This patch also fixes encoding of hidden fields used in sort by form. And lastly, it tries to make facet changes for opac and intranet as same as possible to simplify future maintencence of this code. Test scenario: 1. find results in your opac which contain accented characters 2. click on them and verify that results are missing 3. apply this patch 4. re-run search and click on facets link verifying that there are now results 5. test sort by form and verify that results are ok 6. verify that facets are still safe from injection by constructing url like /cgi-bin/koha/opac-search.pl?q=123&sort_by='"><script>prompt('Happy_Holidays')</script>&limit=123 and verifying that you DON'T see prompt window in your browser Sorry, failing qa again. The title of the [X] link (to delete the facet) is badly encoded. Created attachment 36611 [details] [review] Bug 13789 - facets with accented utf-8 characters generate double encoded links Bug 13425 tried to fix XSS in OPAC, by using url filter in template toolkit on whole generated url. This doesn't work and create double encoded strings in facets because we are creating url variable by concatenating query_cgi (which did pass through uri_escape_utf8 on perl side) and other parameters which have to be escaped in template. Also, code like [% SET limit_cgi_f = limit_cgi | url %] doesn't do anything (at least doesn't apply url filter) so it's not needed. This patch also fixes encoding of hidden fields used in sort by form. And lastly, it tries to make facet changes for opac and intranet as same as possible to simplify future maintencence of this code. Test scenario: 1. find results in your opac which contain accented characters 2. click on them and verify that results are missing 3. apply this patch 4. re-run search and click on facets link verifying that there are now results 5. test sort by form and verify that results are ok 6. verify that facets are still safe from injection by constructing url like /cgi-bin/koha/opac-search.pl?q=123&sort_by='"><script>prompt('Happy_Holidays')</script>&limit=123 and verifying that you DON'T see prompt window in your browser Created attachment 36616 [details] [review] Bug 13789 - facets with accented utf-8 characters generate double encoded links Bug 13425 tried to fix XSS in OPAC, by using url filter in template toolkit on whole generated url. This doesn't work and create double encoded strings in facets because we are creating url variable by concatenating query_cgi (which did pass through uri_escape_utf8 on perl side) and other parameters which have to be escaped in template. Also, code like [% SET limit_cgi_f = limit_cgi | url %] doesn't do anything (at least doesn't apply url filter) so it's not needed. This patch also fixes encoding of hidden fields used in sort by form. And lastly, it tries to make facet changes for opac and intranet as same as possible to simplify future maintencence of this code. Test scenario: 1. find results in your opac which contain accented characters 2. click on them and verify that results are missing 3. apply this patch 4. re-run search and click on facets link verifying that there are now results 5. test sort by form and verify that results are ok 6. verify that facets are still safe from injection by constructing url like /cgi-bin/koha/opac-search.pl?q=123&sort_by='"><script>prompt('Happy_Holidays')</script>&limit=123 and verifying that you DON'T see prompt window in your browser Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Bumping the severity since it's a regression. Created attachment 36638 [details] [review] [PASSED QA] Bug 13789 - facets with accented utf-8 characters generate double encoded links Bug 13425 tried to fix XSS in OPAC, by using url filter in template toolkit on whole generated url. This doesn't work and create double encoded strings in facets because we are creating url variable by concatenating query_cgi (which did pass through uri_escape_utf8 on perl side) and other parameters which have to be escaped in template. Also, code like [% SET limit_cgi_f = limit_cgi | url %] doesn't do anything (at least doesn't apply url filter) so it's not needed. This patch also fixes encoding of hidden fields used in sort by form. And lastly, it tries to make facet changes for opac and intranet as same as possible to simplify future maintencence of this code. Test scenario: 1. find results in your opac which contain accented characters 2. click on them and verify that results are missing 3. apply this patch 4. re-run search and click on facets link verifying that there are now results 5. test sort by form and verify that results are ok 6. verify that facets are still safe from injection by constructing url like /cgi-bin/koha/opac-search.pl?q=123&sort_by='"><script>prompt('Happy_Holidays')</script>&limit=123 and verifying that you DON'T see prompt window in your browser Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Patch pushed to master. Thanks Dobrica! Pushed to 3.18.x will be in 3.18.5 (In reply to Chris Cormack from comment #12) > Pushed to 3.18.x will be in 3.18.5 It introduces regression in 3.18.x, the facet is not selected anymore. To reproduce: - At the opac, search for something - click on the author facet '<a>Goupy, Xavier</a>' - the facet is correctly selected (Goupy, Xavier [<a>x</a>]) - search for something again - click on the author facet '<a>Leblanc, Stéphane</a>' - the facet is not correctly selected (<a>Leblanc, Stéphane</a>) Note: Without this patch, the only issue on facet is the title of the [x] link (<a title="Remove facet Leblanc%2C%20St%C3%A9phane">x</a>) Good catch, reverted |