From 53bb55704014883af44b546cb234b8521e2422dd 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 | 12 ++-- .../prog/en/modules/reports/dictionary.tt | 10 +++ reports/dictionary.pl | 64 +++++++++++-------- 3 files changed, 55 insertions(+), 31 deletions(-) diff --git a/C4/Reports/Guided.pm b/C4/Reports/Guided.pm index f5f65d9639b..62ca35740f3 100644 --- a/C4/Reports/Guided.pm +++ b/C4/Reports/Guided.pm @@ -219,11 +219,13 @@ sub _get_columns { return [ map { my $column_name = $_->{Field}; - { - name => sprintf( "%s.%s", $table_name, $column_name ), - description => $all_columns->{$table_name}->{$column_name}, - }; - + my $forbidden = Koha::Report->new->check_columns( undef, [ $column_name ] ); + $forbidden ? () : ( + { + name => sprintf( "%s.%s", $table_name, $column_name ), + description => $all_columns->{$table_name}->{$column_name}, + } + ); } @$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 d30ade2126e..47d6eb505fa 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/dictionary.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/dictionary.tt @@ -150,6 +150,16 @@

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 1 of 5: Name the new definition
    diff --git a/reports/dictionary.pl b/reports/dictionary.pl index 1446c0718a9..86bbf1e5fdc 100755 --- a/reports/dictionary.pl +++ b/reports/dictionary.pl @@ -79,38 +79,50 @@ if ( $op eq 'add_form' ) { # Choosing the values 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; + 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; } - if ( $type eq 'DATE' || $type eq 'DATETIME' ) { - $tmp_hash{'date'} = 1; - } - if ( $type eq 'TEXT' || $type eq 'MEDIUMTEXT' ) { - $tmp_hash{'text'} = 1; - } - - # 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, + 'definition_name' => $definition_name, + 'definition_description' => $definition_description, + 'columns' => \@column_loop, + 'columnstring' => $columnstring, + 'passworderr' => 1, + ); + } } elsif ( $op eq 'cud-add_form_5' ) { -- 2.39.5