gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
This commit is contained in:
Dobrica Pavlinusic 2015-03-04 11:38:42 +01:00 committed by Tomas Cohen Arazi
parent 07e6ddc44a
commit 1ca9adaa56
4 changed files with 14 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
catalogue/search.pl
View File

@ -519,7 +519,7 @@ for my $this_cgi ( split('&',$limit_cgi) ) {
my $input_name = $1;
my $input_value = $2;
$input_name =~ s/=$//;
push @limit_inputs, { input_name => $input_name, input_value => uri_unescape($input_value) };
push @limit_inputs, { input_name => $input_name, input_value => Encode::decode_utf8( uri_unescape($input_value) ) };
}
$template->param ( LIMIT_INPUTS => \@limit_inputs );
@ -723,12 +723,11 @@ my $gotopage = $cgi->param('gotoPage');
$template->{'VARS'}->{'gotoPage'} = $gotopage
if $gotopage =~ m/^(ISBD|labeledMARC|MARC|more)?detail.pl$/;
my @input_values = map { Encode::decode_utf8($_->{input_value}) } @limit_inputs;
for my $facet ( @$facets ) {
for my $entry ( @{ $facet->{facets} } ) {
my $index = $entry->{type_link_value};
my $value = $entry->{facet_link_value};
$entry->{active} = grep { $_ eq qq{$index:$value} } @input_values;
$entry->{active} = grep { $_->{input_value} eq qq{$index:$value} } @limit_inputs;
}
}

14
koha-tmpl/intranet-tmpl/prog/en/includes/facets.inc
View File

@ -23,17 +23,14 @@
<ul>
[% FOREACH facet IN facets_loo.facets %]
<li>
[% SET query_cgi_f = query_cgi | html %]
[% SET limit_cgi_f = limit_cgi | html %]
[% SET url = "/cgi-bin/koha/catalogue/search.pl?" _ query_cgi_f _ limit_cgi_f %]
[% SET url = "/cgi-bin/koha/catalogue/search.pl?" _ query_cgi _ limit_cgi %]
[% IF ( sort_by ) %]
[% SET url = url _ "&amp;sort_by=" _ sort_by %]
[% url = BLOCK %][% url %][% "&amp;sort_by=" _ sort_by |url %][% END %]
[% END %]
[% facet.facet_link_value = BLOCK %][% facet.facet_link_value | uri %][% END %]
[% IF facet.active %]
[% SET url = url _ "&amp;nolimit=" _ facet.type_link_value _ ":" _ facet.facet_link_value %]
<span class="facet-label">[% facet.facet_label_value %]</span>
[<a href="[% url |url%]" title="Remove facet [% facet.facet_title_value | html %]">x</a>]
[<a href="[% url %]" title="Remove facet [% facet.facet_link_value | html %]">x</a>]
[% ELSE %]
[% SET url = url _ "&amp;limit=" _ facet.type_link_value _ ":" _ facet.facet_link_value %]
<span class="facet-label"><a href="[% url %]" title="[% facet.facet_title_value |html %]">[% facet.facet_label_value %]</a></span>
@ -44,7 +41,10 @@
</li>
[% END %]
[% IF ( facets_loo.expandable ) %]
<li class="showmore"><a href="/cgi-bin/koha/catalogue/search.pl?[% query_cgi |html %][% limit_cgi |html %][% IF ( sort_by ) %]&amp;sort_by=[% sort_by |url%][% END %][% IF ( offset ) %]&amp;offset=[% offset %][% END %]&amp;expand=[% facets_loo.expand %]#[% facets_loo.type_id %]">Show more</a></li>
<li class="showmore">
<a href="/cgi-bin/koha/catalogue/search.pl?[% query_cgi %][% limit_cgi |url %][% IF ( sort_by ) %]&amp;sort_by=[% sort_by |url %][% END %][% IF ( offset ) %]
&amp;offset=[% offset |url %][% END %]&amp;expand=[% facets_loo.expand |url %]#[% facets_loo.type_id |url %]">Show more</a>
</li>
[% END %]
</ul></li>
[% END %]

11
koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-facets.inc
View File

@ -32,20 +32,17 @@
<ul>
[% FOREACH facet IN facets_loo.facets %]
<li>
[% SET query_cgi_f = query_cgi %]
[% SET limit_cgi_f = limit_cgi | url %]
[% SET url = "/cgi-bin/koha/opac-search.pl?" _ query_cgi_f _ limit_cgi_f %]
[% SET url = "/cgi-bin/koha/opac-search.pl?" _ query_cgi _ limit_cgi %]
[% IF ( sort_by ) %]
[% SET url = url _ "&amp;sort_by=" _ sort_by |url %]
[% url = BLOCK %][% url %][% "&amp;sort_by=" _ sort_by |url %][% END %]
[% END %]
[% facet.facet_link_value = BLOCK %][% facet.facet_link_value | uri %][% END %]
[% IF facet.active %]
[% SET url = url _ "&amp;nolimit=" _ facet.type_link_value _ ":" _ facet.facet_link_value %]
<span class="facet-label">[% facet.facet_label_value %]</span>
[<a href="[% url |url%]" title="Remove facet [% facet.facet_link_value | html %]">x</a>]
[<a href="[% url %]" title="Remove facet [% facet.facet_link_value | html %]">x</a>]
[% ELSE %]
[% SET url = url _ "&amp;limit=" _ facet.type_link_value _ ":" _ facet.facet_link_value %]
<span class="facet-label"><a href="[% url |url%]" title="[% facet.facet_title_value |html %]">[% facet.facet_label_value %]</a></span>
<span class="facet-label"><a href="[% url %]" title="[% facet.facet_title_value |html %]">[% facet.facet_label_value %]</a></span>
[% IF ( displayFacetCount ) %]
<span class="facet-count"> ([% facet.facet_count %])</span>
[% END %]

3
opac/opac-search.pl
View File

@ -872,12 +872,11 @@ for (my $i=0;$i<@servers;$i++) {
} #/end of the for loop
#$template->param(FEDERATED_RESULTS => \@results_array);
my @input_values = map { Encode::decode_utf8($_->{input_value}) } @limit_inputs;
for my $facet ( @$facets ) {
for my $entry ( @{ $facet->{facets} } ) {
my $index = $entry->{type_link_value};
my $value = $entry->{facet_link_value};
$entry->{active} = grep { $_ eq qq{$index:$value} } @input_values;
$entry->{active} = grep { $_->{input_value} eq qq{$index:$value} } @limit_inputs;
}
}