Bug 22478: Prevent XSS vulnerabilities when pagination appears
authorJonathan Druart <jonathan.druart@bugs.koha-community.org>
Thu, 14 Mar 2019 22:42:50 +0000 (19:42 -0300)
committerLucas Gass <lucas@bywatersolutions.com>
Mon, 29 Apr 2019 01:29:06 +0000 (01:29 +0000)
This is a bad one as we thought we were XSS safe since bug 13618.

The html code generated in C4::Output::pagination_bar must escape the
variables and values correctly.

This patch needs to be widely tested, everywhere the pagination appears,
to make sure we will not introduce regressions.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
(cherry picked from commit d4d1107afa873614ace241557e424de0dcbad20a)

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

C4/Output.pm

index 71f82e8..e8a86cc 100644 (file)
@@ -29,6 +29,7 @@ use strict;
 #use warnings; FIXME - Bug 2505
 
 use URI::Escape;
+use Scalar::Util qw( looks_like_number );
 
 use C4::Context;
 use C4::Templates;
@@ -89,6 +90,9 @@ sub pagination_bar {
     my $startfrom_name = (@_) ? shift : 'page';
     my $additional_parameters = shift || {};
 
+    $current_page = looks_like_number($current_page) ? $current_page : undef;
+    $nb_pages     = looks_like_number($nb_pages)     ? $nb_pages     : undef;
+
     # how many pages to show before and after the current page?
     my $pages_around = 2;
 
@@ -106,7 +110,7 @@ sub pagination_bar {
     my $url = $base_url . (($base_url =~ m/$delim/ or $base_url =~ m/\?/) ? '&amp;' : '?' ) . $startfrom_name . '=';
     my $url_suffix;
     while ( my ( $k, $v ) = each %$additional_parameters ) {
-        $url_suffix .= '&amp;' . $k . '=' . $v;
+        $url_suffix .= '&amp;' . URI::Escape::uri_escape_utf8($k) . '=' . URI::Escape::uri_escape_utf8($v);
     }
     my $pagination_bar = '';