Bugzilla – Attachment 179567 Details for
Bug 37784
Patron password hash can be fetched using report dictionary
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 37784: [24.05] Do not show forbidden columns in reports dictionary
Bug-37784-2405-Do-not-show-forbidden-columns-in-re.patch (text/plain), 6.49 KB, created by
Aleisha Amohia
on 2025-03-21 01:27:40 UTC
(
hide
)
Description:
Bug 37784: [24.05] Do not show forbidden columns in reports dictionary
Filename:
MIME Type:
Creator:
Aleisha Amohia
Created:
2025-03-21 01:27:40 UTC
Size:
6.49 KB
patch
obsolete
>From 0676a0ed7d02bc4e48825236bf3fb9dc35729aee Mon Sep 17 00:00:00 2001 >From: Aleisha Amohia <aleishaamohia@hotmail.com> >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: > ><option value="borrowers.password">Card number</option> > >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 <dcook@prosentient.com.au> > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >--- > 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 @@ > <h3>Add new definition</h3> > <form action="/cgi-bin/koha/reports/dictionary.pl" method="post"> > [% INCLUDE 'csrf-token.inc' %] >+ >+ [% IF ( passworderr ) %] >+ <div class="alert alert-warning"> >+ <strong>The following error was encountered:</strong><br /> >+ The previous column selection included a password field.<br /> >+ The column cannot be used due to security risks.<br /> >+ Please make a new selection and ensure no password columns have been selected. >+ </div> >+ [% END %] >+ > <fieldset class="rows"><legend>Step 1 of 5: Name the new definition</legend><ol> > <li> > <label for="definition_name">Definition name:</label> >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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 37784
:
170898
|
171041
|
174817
|
177824
|
179297
|
179312
|
179313
|
179314
| 179567 |
179568
|
179569