Hi Jonathan, Sorry, I wasn't precise in my language. I know that we already use \Q...\E in Koha, but I wonder how many places in Koha we are passing variables into regular expressions without considering whether or not we might need it there. I don't know the contexts but here are some examples: authorities/authorities-home.pl: $query_cgi_history =~ s/^$path_info\?//; C4/Patroncards/Patroncard.pm: $_ =~ s/$_/$borrower_attributes->{$field}/; C4/Labels/Label.pm: elsif ( $f =~ /^($match_kohatable).*/ ) { C4/External/Syndetics.pm: if (exists $response->{$available_type} && $response->{$available_type} =~ /$available_type/) { Maybe by convention most of these won't be a problem but some of them might be. That $borrower_attributes->{$field} one could possibly be user-entered data. It could be interesting trying to craft a malicious regex to inject into that line. Most cases probably wouldn't be too problematic. This one is problematic because it's inability to work can cause infinite loops which can bring down web servers or potentially even entire servers. C4/Patroncards/Patroncard.pm: $line =~ s/$1//; I'm just thinking that maybe we should be more careful with what we're feeding into regular expressions. (Although the infinite loop is actually indicative of other problems with C4/Patroncards/Patroncard.pm...) David Cook Systems Librarian Prosentient Systems 72/330 Wattle St Ultimo, NSW 2007 Australia Office: 02 9212 0899 Direct: 02 8005 0595 -----Original Message----- From: Jonathan Druart [mailto:jonathan.druart@bugs.koha-community.org] Sent: Thursday, 7 March 2019 3:01 AM To: David Cook <dcook@prosentient.com.au> Cc: koha-devel <koha-devel@lists.koha-community.org> Subject: Re: [Koha-devel] Using unsanitized data in regular expressions We already use it in several places. I found 28 occurrences in the current code. Le lun. 4 mars 2019 à 04:00, David Cook <dcook@prosentient.com.au> a écrit :
Hi all,
I encountered a problem recently where create-pdf.pl was stuck in an infinite loop because a string passed into ‘$line =~ s/$1//;’ in C4::Patroncards::Patroncard contained “(TEST)” and it was treating the parentheses as metacharacters and not literal values.
I fixed the problem by changing the line to ‘$line =~ s/\Q$1\E//;’, so that anything in $1 would be escaped, but it got me thinking about other parts of Koha. I’m not saying that it’s necessarily a problem in other parts of Koha, but that it might be good to be mindful of it.
David Cook
Systems Librarian
Prosentient Systems
72/330 Wattle St
Ultimo, NSW 2007
Australia
Office: 02 9212 0899
Direct: 02 8005 0595
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/