gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 37508: (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>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This commit is contained in:
Marcel de Rooy 2024-08-09 09:50:44 +00:00 committed by Katrin Fischer
parent ed6cb0f274
commit aebc7ba8db
No known key found for this signature in database
GPG Key ID: 0EF6E2C03357A834
2 changed files with 67 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
Koha/Report.pm
View File

@ -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

29
t/db_dependent/Koha/Reports.t
View File

@ -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;