Bugzilla – Attachment 170245 Details for
Bug 37508
SQL reports should not show patron password hash if queried
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 37508: [23.11] (QA follow-up) Move check to Koha::Report, extend
Bug-37508-2311-QA-follow-up-Move-check-to-KohaRepo.patch (text/plain), 4.23 KB, created by
Aleisha Amohia
on 2024-08-12 21:20:19 UTC
(
hide
)
Description:
Bug 37508: [23.11] (QA follow-up) Move check to Koha::Report, extend
Filename:
MIME Type:
Creator:
Aleisha Amohia
Created:
2024-08-12 21:20:19 UTC
Size:
4.23 KB
patch
obsolete
>From 08494a901caddc9247d0b21846892add97aa4eb2 Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Fri, 9 Aug 2024 09:50:44 +0000 >Subject: [PATCH] Bug 37508: [23.11] (QA follow-up) Move check to Koha::Report, > extend > >Do not allow password but allow password_expiry_days etc. >Do not allow token, secret and uuid too. > >Test plan: >Run t/db_dependent/Koha/Reports.t > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >--- > Koha/Report.pm | 42 +++++++++++++++++++++++++++++++++++ > t/db_dependent/Koha/Reports.t | 29 ++++++++++++++++++++---- > 2 files changed, 67 insertions(+), 4 deletions(-) > >diff --git a/Koha/Report.pm b/Koha/Report.pm >index 55daece8362..78fcf4bb263 100644 >--- a/Koha/Report.pm >+++ b/Koha/Report.pm >@@ -28,6 +28,10 @@ use base qw(Koha::Object); > # Only 1 error is returned > # TODO Koha::Report->store should check this before saving > >+use constant FORBIDDEN_COLUMN_MATCHES => qw(password token uuid secret); >+use constant WHITELISTED_COLUMN_NAMES => >+ qw(password_expiration_date password_expiry_days reset_password change_password min_password_length require_strong_password password_expiration_date); >+ > =head1 NAME > > Koha::Report - Koha Report Object class >@@ -58,11 +62,49 @@ sub is_sql_valid { > push @errors, { sqlerr => $1 }; > } elsif ($sql !~ /^\s*SELECT\b\s*/i) { > push @errors, { queryerr => 'Missing SELECT' }; >+ } else { >+ push @errors, { passworderr => "Illegal column in results" } >+ if $self->check_columns($sql); > } > > return ( @errors ? 0 : 1, \@errors ); > } > >+=head3 check_columns >+ >+ $self->check_columns('SELECT password from borrowers'); >+ $self->check_columns( undef, [qw(password token)] ); >+ >+ Check if passed sql statement or column_names arrayref contains >+ forbidden column names (credentials etc.). >+ Returns all bad column names or empty list. >+ >+=cut >+ >+sub check_columns { >+ my ( $self, $sql, $column_names ) = @_; >+ >+ # TODO When passing sql, we actually go thru the whole statement, not just columns >+ >+ my $regex; >+ push my @columns, @{ $column_names // [] }; >+ if ($sql) { >+ >+ # Now find all matches for e.g. password but also modulepassword or password_hash >+ # Note that \w includes underscore, not hyphen >+ $regex = '(\w*(?:' . ( join '|', FORBIDDEN_COLUMN_MATCHES ) . ')\w*)'; >+ @columns = ( $sql =~ /$regex/ig ); >+ } else { >+ $regex = '(' . ( join '|', FORBIDDEN_COLUMN_MATCHES ) . ')'; >+ @columns = grep { /$regex/i } @columns; >+ } >+ >+ # Check for whitelisted variants >+ $regex = '(^' . ( join '|', WHITELISTED_COLUMN_NAMES ) . ')$'; # should be full match >+ @columns = grep { !/$regex/i } @columns; >+ return @columns; >+} >+ > =head3 get_search_info > > Return search info >diff --git a/t/db_dependent/Koha/Reports.t b/t/db_dependent/Koha/Reports.t >index 5db4d3bae90..3f7eaacd6c6 100755 >--- a/t/db_dependent/Koha/Reports.t >+++ b/t/db_dependent/Koha/Reports.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 6; >+use Test::More tests => 7; > > use Koha::Report; > use Koha::Reports; >@@ -76,8 +76,6 @@ subtest 'prep_report' => sub { > is_deeply( $headers, { 'itemnumber for batch' => 'itemnumber' } ); > }; > >-$schema->storage->txn_rollback; >- > subtest 'is_sql_valid' => sub { > plan tests => 3 + 6 * 2; > my @badwords = ( 'UPDATE', 'DELETE', 'DROP', 'INSERT', 'SHOW', 'CREATE' ); >@@ -115,4 +113,27 @@ subtest 'is_sql_valid' => sub { > $word . ' qux' > ); > } >- } >+}; >+ >+subtest 'check_columns' => sub { >+ plan tests => 3; >+ >+ my $report = Koha::Report->new; >+ is_deeply( [ $report->check_columns('SELECT passWorD from borrowers') ], ['passWorD'], 'Bad column found in SQL' ); >+ is( scalar $report->check_columns('SELECT reset_passWorD from borrowers'), 0, 'No bad column found in SQL' ); >+ >+ is_deeply( >+ [ >+ $report->check_columns( >+ undef, >+ [ >+ qw(change_password hash secret test place mytoken hersecret password_expiry_days password_expiry_days2) >+ ] >+ ) >+ ], >+ [qw(secret mytoken hersecret password_expiry_days2)], >+ 'Check column_names parameter' >+ ); >+}; >+ >+$schema->storage->txn_rollback; >-- >2.39.2
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 37508
:
169806
|
169810
|
170121
|
170124
|
170125
|
170126
|
170127
|
170165
|
170171
|
170172
|
170173
|
170174
|
170175
|
170176
|
170229
|
170230
|
170241
|
170242
|
170243
|
170244
| 170245 |
170246
|
170247
|
170253