From 15692680330773cca78fa4d053c232109f6a4aec Mon Sep 17 00:00:00 2001 From: Aleisha Amohia Date: Fri, 30 Aug 2024 02:04:48 +0000 Subject: [PATCH] Bug 37784: Do not show forbidden columns in reports dictionary This patch prevents forbidden columns from being selected using the reports dictionary. To test: 1. Go to Reports -> View dictionary 2. Choose 'new dictionary' 3. After providing a name and description, select the 'patrons' table 4. Select the 'password' column and go Next 5. Notice it is possible to see a list of all the password hashes in the 'choose' selection box when asked to specify a value 6. Apply patch and restart services 7. Repeat steps 1-3 8. Notice the 'password' column and other forbidden columns are not available to be selected 9. Open the Inspector / developer tools in your browser and find the code for the columns selection 10. Change the value of one of the options in the code to "borrowers.password". So it may end up looking like this: 11. Choose that option from the dropdown and click Next. 12. Confirm you are redirected back to this page and warned about the column being used. Sponsored-by: Reserve Bank of New Zealand Signed-off-by: David Cook --- C4/Reports/Guided.pm | 11 +-- .../prog/en/modules/reports/dictionary.tt | 8 +++ reports/dictionary.pl | 67 +++++++++++-------- 3 files changed, 54 insertions(+), 32 deletions(-) diff --git a/C4/Reports/Guided.pm b/C4/Reports/Guided.pm index 1f1dd80ca2..6f53982437 100644 --- a/C4/Reports/Guided.pm +++ b/C4/Reports/Guided.pm @@ -220,10 +220,13 @@ sub _get_columns { $tablehash{'__first__'} = $first; push @columns, \%tablehash; while ( my $data = $sth->fetchrow_arrayref() ) { - my %temphash; - $temphash{'name'} = "$tablename.$data->[0]"; - $temphash{'description'} = $columns->{$tablename}->{$data->[0]}; - push @columns, \%temphash; + my $forbidden = Koha::Report->new->check_columns( undef, $data ); + unless ($forbidden) { + my %temphash; + $temphash{'name'} = "$tablename.$data->[0]"; + $temphash{'description'} = $columns->{$tablename}->{ $data->[0] }; + push @columns, \%temphash; + } } $sth->finish(); return (@columns); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/dictionary.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/dictionary.tt index 5b1ad7c348..39d2b5f791 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/dictionary.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/dictionary.tt @@ -184,6 +184,14 @@

Add new definition

[% INCLUDE 'csrf-token.inc' %] +[% IF ( passworderr ) %] +
+ The following error was encountered:
+ The previous column selection included a password field.
+ The column cannot be used due to security risks.
+ Please make a new selection and ensure no password columns have been selected. +
+[% END %]
Step 3 of 5: Choose a column diff --git a/reports/dictionary.pl b/reports/dictionary.pl index 123dab5340..bd1cc8bd89 100755 --- a/reports/dictionary.pl +++ b/reports/dictionary.pl @@ -80,39 +80,50 @@ elsif ( $op eq 'cud-add_form_3' ) { elsif ( $op eq 'cud-add_form_4' ) { # Choosing the values - my @columns = $input->multi_param('columns'); - my $columnstring = join( ',', @columns ); + my @columns = $input->multi_param('columns'); + my $columnstring = join( ',', @columns ); + my $forbidden = Koha::Report->new->check_columns( undef, \@columns ); my @column_loop; foreach my $column (@columns) { - my %tmp_hash; - $tmp_hash{'name'} = $column; - my $type = get_column_type($column); - if ( $type eq 'distinct' ) { - my $values = get_distinct_values($column); - $tmp_hash{'values'} = $values; - $tmp_hash{'distinct'} = 1; - - } - if ( $type eq 'DATE' || $type eq 'DATETIME' ) { - $tmp_hash{'date'} = 1; - } - if ($type eq 'TEXT' || $type eq 'MEDIUMTEXT'){ - $tmp_hash{'text'} = 1; + unless ($forbidden) { + my %tmp_hash; + $tmp_hash{'name'} = $column; + my $type = get_column_type($column); + if ( $type eq 'distinct' ) { + my $values = get_distinct_values($column); + $tmp_hash{'values'} = $values; + $tmp_hash{'distinct'} = 1; + } + if ( $type eq 'DATE' || $type eq 'DATETIME' ) { + $tmp_hash{'date'} = 1; + } + if ( $type eq 'TEXT' || $type eq 'MEDIUMTEXT' ) { + $tmp_hash{'text'} = 1; + } + push @column_loop, \%tmp_hash; } - - # else { - # warn $type;# - # } - push @column_loop, \%tmp_hash; } - $template->param( 'step_4' => 1, - 'area' => $area, - 'definition_name' => $definition_name, - 'definition_description' => $definition_description, - 'columns' => \@column_loop, - 'columnstring' => $columnstring, - ); + if (@column_loop) { + $template->param( + 'step_4' => 1, + 'area' => $area, + 'definition_name' => $definition_name, + 'definition_description' => $definition_description, + 'columns' => \@column_loop, + 'columnstring' => $columnstring, + ); + } else { + my $columns = get_columns( $area, $input ); + $template->param( + 'step_3' => 1, + 'area' => $area, + 'columns' => $columns, + 'definition_name' => $definition_name, + 'definition_description' => $definition_description, + 'passworderr' => 1, + ); + } } elsif ( $op eq 'cud-add_form_5' ) { -- 2.39.5