[Koha-devel] Using unsanitized data in regular expressions

[David Cook]

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 at bugs.koha-community.org] 
Sent: Thursday, 7 March 2019 3:01 AM
To: David Cook <dcook at prosentient.com.au>
Cc: koha-devel <koha-devel at 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 at 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 at 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/