From 0676a0ed7d02bc4e48825236bf3fb9dc35729aee Mon Sep 17 00:00:00 2001 From: Aleisha Amohia Date: Fri, 30 Aug 2024 02:04:48 +0000 Subject: [PATCH] Bug 37784: [24.05] 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. 13. Ensure tests pass t/db_dependent/Reports/Guided.t Sponsored-by: Reserve Bank of New Zealand Signed-off-by: David Cook Signed-off-by: Marcel de Rooy --- C4/Reports/Guided.pm | 11 ++-- .../prog/en/modules/reports/dictionary.tt | 10 +++ reports/dictionary.pl | 66 +++++++++++-------- 3 files changed, 57 insertions(+), 30 deletions(-) diff --git a/C4/Reports/Guided.pm b/C4/Reports/Guided.pm index 1f1dd80ca23..fe717d097d3 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->[0]] ); + 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 facf3987bec..568c0f817db 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/dictionary.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/dictionary.tt @@ -140,6 +140,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
  1. diff --git a/reports/dictionary.pl b/reports/dictionary.pl index 123dab53409..fdab74a94ce 100755 --- a/reports/dictionary.pl +++ b/reports/dictionary.pl @@ -80,39 +80,53 @@ 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; + 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; + 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, + ); } - $template->param( 'step_4' => 1, - 'area' => $area, - 'definition_name' => $definition_name, - 'definition_description' => $definition_description, - 'columns' => \@column_loop, - 'columnstring' => $columnstring, - ); } elsif ( $op eq 'cud-add_form_5' ) { -- 2.39.5