From 8659e59b7825aa5eae6ca02844849d1553d634eb Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Thu, 27 Jul 2023 09:37:39 +0000 Subject: [PATCH] Bug 34349: Validate input for scheduler Content-Type: text/plain; charset=utf-8 Test plan: Try various wrong inputs via URL or form. See other report for hacking format with backtics. Run t/Koha/Validator.t --- Koha/Validator.pm | 57 ++++++++++++++++++ .../prog/en/modules/tools/scheduler.tt | 4 ++ t/Koha/Validator.t | 58 +++++++++++++++++++ tools/scheduler.pl | 50 +++++++++++----- 4 files changed, 153 insertions(+), 16 deletions(-) create mode 100644 Koha/Validator.pm create mode 100755 t/Koha/Validator.t diff --git a/Koha/Validator.pm b/Koha/Validator.pm new file mode 100644 index 0000000000..e343ee5a35 --- /dev/null +++ b/Koha/Validator.pm @@ -0,0 +1,57 @@ +package Koha::Validator; + +#TODO License + +use Modern::Perl; +use List::Util qw(any); +use Koha::Exceptions; +use Koha::Email; + +our $default_rules = { + digits_only => qr/^\d+$/, + email => \&_check_email, +}; + +#TODO POD + +sub new { + my ( $class, $params ) = @_; + my $self = bless $params // {}, $class; + return $self; +} + +sub load { + my ( $self, $rules ) = @_; + $self->{rules} = $rules if ref($rules) eq 'HASH'; +} + +sub check_array { #TODO Add later +} + +sub check_hash { + my ( $self, $values ) = @_; + return if ref($values) ne 'HASH'; + + foreach my $key (%$values) { + my $rule = $self->{rules}->{$key} or next; + $rule = $default_rules->{$rule} if ref($rule) eq ''; + if ( ref($rule) eq 'Regexp' ) { + next if $values->{$key} =~ $rule; + } elsif ( ref($rule) eq 'CODE' ) { + next if $rule->( $values->{$key} ); + } elsif ( ref($rule) eq 'ARRAY' ) { + next if any { $_ eq $values->{$key} } @$rule; + } + Koha::Exceptions::BadParameter->throw( parameter => $key ); + } + return 1; +} + +sub _check_email { # TODO Incorporate into Koha::Email? + my $email = shift; + return 0 if !Koha::Email->is_valid($email); + return 0 if $email =~ /[\`\$]/; + return 1; +} + +1; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/scheduler.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/scheduler.tt index 033d819884..22bc589e36 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/scheduler.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/scheduler.tt @@ -31,8 +31,11 @@ [% IF ( job_add_failed ) %]
Failed to add scheduled task
+[% ELSIF validation_failed %] +
Validation for [% invalid_field | html %] failed
[% END %] +[% UNLESS validation_failed %]
@@ -73,6 +76,7 @@
+[% END %] [% IF ( JOBS ) %]

Jobs already entered

diff --git a/t/Koha/Validator.t b/t/Koha/Validator.t new file mode 100755 index 0000000000..8c4479c772 --- /dev/null +++ b/t/Koha/Validator.t @@ -0,0 +1,58 @@ +#!/usr/bin/perl + +# Copyright 2023 Rijksmuseum +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Test::More tests => 1; +use Test::Warn; +use Try::Tiny; + +use Koha::Validator; + +subtest 'trivial tests' => sub { + plan tests => 3; + + my $rules = { + test01 => 'digits_only', + test02 => [ 1, 2 ], + test03 => sub { 0 }, + test04 => 'email', + test05 => qr/test/, + }; + my $validator = Koha::Validator->new( { rules => $rules } ); + try { + $validator->check_hash( + { test01 => 2, test02 => 1, test04 => 'marcel@test.nl', test05 => 'includes test word' } ); + ok( 1, 'validator green' ); + } catch { + ok(0); + }; + try { + $validator->check_hash( { test01 => 2, test02 => 1, test03 => 'nomatterwhat' } ); + ok( 0, 'we should not be here' ); + } catch { + ok( 1, 'validator red' ); + }; + try { + $validator->check_hash( { test04 => 'withbacktic`ls`@test.com' } ); + ok( 0, 'we should not be here' ); + } catch { + ok( 1, 'validator red for email' ); + }; +}; diff --git a/tools/scheduler.pl b/tools/scheduler.pl index ecee48e930..6d6673102c 100755 --- a/tools/scheduler.pl +++ b/tools/scheduler.pl @@ -18,16 +18,31 @@ # along with Koha; if not, see . use Modern::Perl; +use CGI qw ( -utf8 ); +use Try::Tiny qw(try catch); + use C4::Context; use C4::Scheduler qw( add_at_job get_jobs remove_at_job ); use C4::Reports::Guided qw( get_saved_reports ); use C4::Auth qw( get_template_and_user ); -use CGI qw ( -utf8 ); use C4::Output qw( output_html_with_http_headers ); -use Koha::DateUtils qw( dt_from_string );; + +use Koha::DateUtils qw( dt_from_string ); +use Koha::Validator; my $input = CGI->new; my $base; +my $rules = { + id => 'digits_only', + mode => [ q(), qw(job_add job_change) ], + delete => [qw(Delete)], + job_id => 'digits_only', + starttime => qr/^\d{2}:?\d{2}$/, + startdate => qr/^\d{4}-?\d{2}-?\d{2}$/, + report => 'digits_only', + format => [ q{}, qw( text html csv tsv ) ], + email => 'email' +}; if ( C4::Context->config('supportdir') ) { $base = C4::Context->config('supportdir'); @@ -47,9 +62,22 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( } ); -my $mode = $input->param('mode'); +my $mode = $input->param('mode') // q{}; my $id = $input->param('id'); +try { + Koha::Validator->new({ rules => $rules })->check_hash(scalar $input->Vars); +} catch { + my $field = 'some field'; + if ( ref($_) =~ /Koha::Exception/ ) { + warn $_->full_message; + $field = $_->parameter if $_->parameter; + } + $template->param( validation_failed => 1, invalid_field => $field ); + output_html_with_http_headers $input, $cookie, $template->output; + exit; +}; + if ( $mode eq 'job_add' ) { my $startdate = dt_from_string( scalar $input->param('startdate'), 'iso' )->ymd; @@ -59,23 +87,13 @@ if ( $mode eq 'job_add' ) { my $starttime = $input->param('starttime'); $starttime =~ s/\://g; my $start = $startdate . $starttime; - my $report = $input->param('report'); - my $format = $input->param('format'); - my $email = $input->param('email'); + my $report = $input->param('report') // 0; + my $format = $input->param('format') // q{}; + my $email = $input->param('email') // q{}; my $command = "export KOHA_CONF=\"$CONFIG_NAME\"; " . "$base/cronjobs/runreport.pl $report --format=$format --to=$email"; -#FIXME commit ea899bc55933cd74e4665d70b1c48cab82cd1257 added recurring parameter (it is not in template) and call to add_cron_job (undefined) -# my $recurring = $input->param('recurring'); -# if ($recurring) { -# my $frequency = $input->param('frequency'); -# add_cron_job( $start, $command ); -# } -# else { -# #here was the the unless ( add_at_job -# } - unless ( add_at_job( $start, $command ) ) { $template->param( job_add_failed => 1 ); } -- 2.30.2