From 4c33e13e8d9469262493daa5ade5a1e05a413fb8 Mon Sep 17 00:00:00 2001 From: Jared Camins-Esakov Date: Sat, 9 Mar 2013 09:59:51 -0500 Subject: [PATCH] Bug 9239 QA follow-up: escape CGI input Koha was not previously escaping CGI input, which caused problems for highlighting and is a security issue. Signed-off-by: Katrin Fischer Thx for fixing this. Signed-off-by: Jared Camins-Esakov --- C4/Search.pm | 37 +++++++++++++++++++++---------------- opac/opac-search.pl | 8 +++++++- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/C4/Search.pm b/C4/Search.pm index e856db8..2c3e605 100644 --- a/C4/Search.pm +++ b/C4/Search.pm @@ -1194,11 +1194,16 @@ sub parseQuery { } foreach my $limit (@limits) { } - foreach my $modifier (@sort_by) { - $query .= " #$modifier"; + if (scalar (@sort_by) > 0) { + my $modifier_re = '#(' . join( '|', @{$QParser->modifiers}) . ')'; + $query =~ s/$modifier_re//g; + foreach my $modifier (@sort_by) { + $query .= " #$modifier"; + } } $query_desc = $query; + $query_desc =~ s/\s+/ /g; if ( C4::Context->preference("QueryWeightFields") ) { } $QParser->add_bib1_filter_map( 'biblioserver', 'su-br', { 'callback' => \&_handle_exploding_index }); @@ -1206,9 +1211,9 @@ sub parseQuery { $QParser->add_bib1_filter_map( 'biblioserver', 'su-rl', { 'callback' => \&_handle_exploding_index }); $QParser->parse( $query ); $operands[0] = "pqf=" . $QParser->target_syntax('biblioserver'); -# TODO: once we are using QueryParser, all this special case code for -# exploded search indexes will be replaced by a callback to -# _handle_exploding_index + } else { + my $modifier_re = '#(' . join( '|', @{Koha::QueryParser::Driver::PQF->modifiers}) . ')'; + s/$modifier_re//g for @operands; } return ( $operators, \@operands, $indexes, $limits, $sort_by, $scan, $lang, $query_desc); @@ -1292,17 +1297,17 @@ sub buildQuery { if ( @limits ) { $q .= ' and '.join(' and ', @limits); } - return ( undef, $q, $q, "q=ccl=$q", $q, '', '', '', '', 'ccl' ); + return ( undef, $q, $q, "q=ccl=".uri_escape($q), $q, '', '', '', '', 'ccl' ); } if ( $query =~ /^cql=/ ) { - return ( undef, $', $', "q=cql=$'", $', '', '', '', '', 'cql' ); + return ( undef, $', $', "q=cql=".uri_escape($'), $', '', '', '', '', 'cql' ); } if ( $query =~ /^pqf=/ ) { if ($query_desc) { - $query_cgi = "q=$query_desc"; + $query_cgi = "q=".uri_escape($query_desc); } else { $query_desc = $'; - $query_cgi = "q=pqf=$'"; + $query_cgi = "q=pqf=".uri_escape($'); } return ( undef, $', $', $query_cgi, $query_desc, '', '', '', '', 'pqf' ); } @@ -1474,9 +1479,9 @@ sub buildQuery { $query .= " $operators[$i-1] "; $query .= " $index_plus " unless $indexes_set; $query .= " $operand"; - $query_cgi .= "&op=$operators[$i-1]"; - $query_cgi .= "&idx=$index" if $index; - $query_cgi .= "&q=$operands[$i]" if $operands[$i]; + $query_cgi .= "&op=".uri_escape($operators[$i-1]); + $query_cgi .= "&idx=".uri_escape($index) if $index; + $query_cgi .= "&q=".uri_escape($operands[$i]) if $operands[$i]; $query_desc .= " $operators[$i-1] $index_plus $operands[$i]"; } @@ -1486,8 +1491,8 @@ sub buildQuery { $query .= " and "; $query .= "$index_plus " unless $indexes_set; $query .= "$operand"; - $query_cgi .= "&op=and&idx=$index" if $index; - $query_cgi .= "&q=$operands[$i]" if $operands[$i]; + $query_cgi .= "&op=and&idx=".uri_escape($index) if $index; + $query_cgi .= "&q=".uri_escape($operands[$i]) if $operands[$i]; $query_desc .= " and $index_plus $operands[$i]"; } } @@ -1499,8 +1504,8 @@ sub buildQuery { $query .= " $index_plus " unless $indexes_set; $query .= $operand; $query_desc .= " $index_plus $operands[$i]"; - $query_cgi .= "&idx=$index" if $index; - $query_cgi .= "&q=$operands[$i]" if $operands[$i]; + $query_cgi .= "&idx=".uri_escape($index) if $index; + $query_cgi .= "&q=".uri_escape($operands[$i]) if $operands[$i]; $previous_operand = 1; } } #/if $operands diff --git a/opac/opac-search.pl b/opac/opac-search.pl index 6be3333..d2ba9fc 100755 --- a/opac/opac-search.pl +++ b/opac/opac-search.pl @@ -358,10 +358,12 @@ unless (@servers) { # operators include boolean and proximity operators and are used # to evaluate multiple operands my @operators = $cgi->param('op'); +@operators = map { uri_unescape($_) } @operators; # indexes are query qualifiers, like 'title', 'author', etc. They # can be single or multiple parameters separated by comma: kw,right-Truncation my @indexes = $cgi->param('idx'); +@indexes = map { uri_unescape($_) } @indexes; # if a simple index (only one) display the index used in the top search box if ($indexes[0] && !$indexes[1]) { @@ -369,16 +371,20 @@ if ($indexes[0] && !$indexes[1]) { } # an operand can be a single term, a phrase, or a complete ccl query my @operands = $cgi->param('q'); +@operands = map { uri_unescape($_) } @operands; $template->{VARS}->{querystring} = join(' ', @operands); # if a simple search, display the value in the search box if ($operands[0] && !$operands[1]) { - $template->param(ms_value => $operands[0]); + my $ms_query = $operands[0]; + $ms_query =~ s/ #\S+//; + $template->param(ms_value => $ms_query); } # limits are use to limit to results to a pre-defined category such as branch or language my @limits = $cgi->param('limit'); +@limits = map { uri_unescape($_) } @limits; if($params->{'multibranchlimit'}) { push @limits, '('.join( " or ", map { "branch: $_ " } @{ GetBranchesInCategory( $params->{'multibranchlimit'} ) } ).')'; -- 1.7.9.5