Attachment 154235 Details for Bug 34349 – Bug 34349: Validate/escape inputs for task scheduler

[patch] Bug 34349: Validate/escape inputs for task scheduler

Bug-34349-Validateescape-inputs-for-task-scheduler.patch (text/plain), 3.53 KB, created by Marcel de Rooy on 2023-08-04 06:30:23 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2023-08-04 06:30:23 UTC

Size: 3.53 KB

patch

obsolete

>From 581e9759e84160994d0757f6c2b9b9d25f241b5e Mon Sep 17 00:00:00 2001
>From: David Cook <dcook@prosentient.com.au>
>Date: Mon, 24 Jul 2023 04:31:15 +0000
>Subject: [PATCH] Bug 34349: Validate/escape inputs for task scheduler
>Content-Type: text/plain; charset=utf-8
>
>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 <jonathan.druart@bugs.koha-community.org>
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>[EDIT] Removed pars for $email =~ regex, removed old commented lines.
>---
> tools/scheduler.pl | 53 ++++++++++++++++++++++++++++++++--------------
> 1 file changed, 37 insertions(+), 16 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";
>-
>-#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 ) ) {
>+    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 ) {
>+
>+        #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 ) ) {
>+            $template->param( job_add_failed => 1 );
>+        }
>+    }
>+    else {
>         $template->param( job_add_failed => 1 );
>     }
> }
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 34349: 153822 | 153955 | 154120 | 154166 | 154235