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

Bug 24695: Improve SQL report validation 

The saved SQL report code validates the SQL in multiple places:
when saving, when updating, and when executing the query.
Move the validation code into Koha::Reports, and write tests for it.

Test plan:
1) Apply patch
2) Create a new valid SQL report, save it (success)
3) Create a new illegal SQL report, try to save (fails)
4) Update already saved SQL report by adding one of
   the forbidden words, eg. delete or drop (saving will fail)
5) Edit a save_sql in the database, changing it to eg.
   "drop borrowers", and try to execute it (fails)
6) Prove t/db_dependent/Koha/Reports.t

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Work as described, no qa errors.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 24695: (QA follow-up) Fix number of tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Pasi Kallinen 2020-02-20 11:31:08 +02:00 committed by Jonathan Druart
parent 925eecd6bd
commit ba5be802e0
4 changed files with 40 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
C4/Reports/Guided.pm
View File

@ -551,11 +551,9 @@ sub execute_query {
$offset = 0 unless $offset;
$limit = 999999 unless $limit;
$debug and print STDERR "execute_query($sql, $offset, $limit)\n";
if ($sql =~ /;?\W?(UPDATE|DELETE|DROP|INSERT|SHOW|CREATE)\W/i) {
return (undef, { sqlerr => $1} );
} elsif ($sql !~ /^\s*SELECT\b\s*/i) {
return (undef, { queryerr => 'Missing SELECT'} );
}
my $errors = Koha::Reports->validate_sql($sql);
return (undef, @{$errors}[0]) if (scalar(@$errors));
foreach my $sql_param ( @$sql_params ){
if ( $sql_param =~ m/\n/ ){

22
Koha/Reports.pm
View File

@ -35,6 +35,28 @@ Koha::Reports - Koha Report Object set class
=cut
=head3 validate_sql
Validate SQL query string so it only contains a select,
not any of the harmful queries.
=cut
sub validate_sql {
my ($self, $sql) = @_;
$sql //= '';
my @errors = ();
if ($sql =~ /;?\W?(UPDATE|DELETE|DROP|INSERT|SHOW|CREATE)\W/i) {
push @errors, { sqlerr => $1 };
} elsif ($sql !~ /^\s*SELECT\b\s*/i) {
push @errors, { queryerr => 'Missing SELECT' };
}
return \@errors;
}
=head3 _type
Returns name of corresponding DBIC resultset

14
reports/guided_reports.pl
View File

@ -239,12 +239,7 @@ elsif ( $phase eq 'Update SQL'){
create_non_existing_group_and_subgroup($input, $group, $subgroup);
if ($sql =~ /;?\W?(UPDATE|DELETE|DROP|INSERT|SHOW|CREATE)\W/i) {
push @errors, {sqlerr => $1};
}
elsif ($sql !~ /^(SELECT)/i) {
push @errors, {queryerr => "No SELECT"};
}
push(@errors, @{Koha::Reports->validate_sql($sql)});
if (@errors) {
$template->param(
@ -600,12 +595,7 @@ elsif ( $phase eq 'Save Report' ) {
create_non_existing_group_and_subgroup($input, $group, $subgroup);
## FIXME this is AFTER entering a name to save the report under
if ($sql =~ /;?\W?(UPDATE|DELETE|DROP|INSERT|SHOW|CREATE)\W/i) {
push @errors, {sqlerr => $1};
}
elsif ($sql !~ /^(SELECT)/i) {
push @errors, {queryerr => "No SELECT"};
}
push(@errors, @{Koha::Reports->validate_sql($sql)});
if (@errors) {
$template->param(

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

@ -17,7 +17,7 @@
use Modern::Perl;
use Test::More tests => 5;
use Test::More tests => 6;
use Koha::Report;
use Koha::Reports;
@ -70,3 +70,15 @@ subtest 'prep_report' => sub {
};
$schema->storage->txn_rollback;
subtest 'validate_sql' => sub {
plan tests => 3 + 6*2;
my @badwords = ('UPDATE', 'DELETE', 'DROP', 'INSERT', 'SHOW', 'CREATE');
is_deeply( Koha::Reports->validate_sql(), [{ queryerr => 'Missing SELECT'}], 'Empty sql is missing SELECT' );
is_deeply( Koha::Reports->validate_sql('FOO'), [{ queryerr => 'Missing SELECT'}], 'Nonsense sql is missing SELECT' );
is_deeply( Koha::Reports->validate_sql('select FOO'), [], 'select FOO is good' );
foreach my $word (@badwords) {
is_deeply( Koha::Reports->validate_sql('select FOO;'.$word.' BAR'), [{ sqlerr => $word}], 'select FOO with '.$word.' BAR' );
is_deeply( Koha::Reports->validate_sql($word.' qux'), [{ sqlerr => $word}], $word.' qux' );
}
}