From dcd698a4b4ed7f3a714ef35fe83c007fce9d52ae Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 24 Jul 2023 04:31:15 +0000 Subject: [PATCH] Bug 34349: Validate/escape inputs for task scheduler This change validates and escapes inputs for task scheduler. Test plan: 0. Apply patch 1. koha-plack --reload kohadev 2. Go to http://localhost:8081/cgi-bin/koha/tools/scheduler.pl 3. Input a time a minute in the future and leave the date blank 4. Choose an existing report and output format 5. Type a malicious string which is also a valid email address into the Email field 6. Click "Save" 7. Note that the job is added but the Email is wrapped in single quotes 8. Try using a non-malicious email address with a single quote. 9. Note that the single quote is escaped, so that it will still be used by runreport.pl JD amended patch: tidy Signed-off-by: Jonathan Druart Signed-off-by: Marcel de Rooy [EDIT] Removed pars for $email =~ regex, removed old commented lines. Signed-off-by: Tomas Cohen Arazi --- tools/scheduler.pl | 49 +++++++++++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/tools/scheduler.pl b/tools/scheduler.pl index ecee48e930..fa4aa516e0 100755 --- a/tools/scheduler.pl +++ b/tools/scheduler.pl @@ -25,6 +25,8 @@ 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::Reports; +use Koha::Email; my $input = CGI->new; my $base; @@ -60,23 +62,42 @@ if ( $mode eq 'job_add' ) { $starttime =~ s/\://g; my $start = $startdate . $starttime; my $report = $input->param('report'); + if ($report) { + my $saved_report; + my $report_id = int($report); + if ($report_id) { + $saved_report = Koha::Reports->find($report_id); + } + if ( !$saved_report ) { + $report = undef; + } + } my $format = $input->param('format'); - my $email = $input->param('email'); - my $command = - "export KOHA_CONF=\"$CONFIG_NAME\"; " . - "$base/cronjobs/runreport.pl $report --format=$format --to=$email"; + if ($format) { + unless ( $format eq 'text' || $format eq 'csv' || $format eq 'html' ) { + $format = undef; + } + } + my $email = $input->param('email'); + if ($email) { + my $is_valid = Koha::Email->is_valid($email); + if ( !$is_valid ) { + $email = undef; + } + } + if ( $report && $format && $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 -# } + #NOTE: Escape any single quotes in email since we're wrapping it in single quotes in bash + $email =~ s/'/'"'"'/g; + my $command = + "export KOHA_CONF=\"$CONFIG_NAME\"; " + . "$base/cronjobs/runreport.pl $report --format=$format --to='$email'"; - unless ( add_at_job( $start, $command ) ) { + unless ( add_at_job( $start, $command ) ) { + $template->param( job_add_failed => 1 ); + } + } + else { $template->param( job_add_failed => 1 ); } }