Bugzilla – Attachment 117081 Details for
Bug 27715
Possibly SQL injection in virtualshelves
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 27715: Use $dbh->quote_identifier to quote untrusted input
Bug-27715-Use-dbh-quoteidentifier-to-quote-untrust.patch (text/plain), 3.26 KB, created by
Martin Renvoize
on 2021-02-19 12:52:44 UTC
(
hide
)
Description:
Bug 27715: Use $dbh->quote_identifier to quote untrusted input
Filename:
MIME Type:
Creator:
Martin Renvoize
Created:
2021-02-19 12:52:44 UTC
Size:
3.26 KB
patch
obsolete
>From 6c219c1e474c5cf14f191c93607ff6a2f31c41ee Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Joonas=20Kylm=C3=A4l=C3=A4?= <joonas.kylmala@helsinki.fi> >Date: Fri, 19 Feb 2021 14:18:15 +0200 >Subject: [PATCH] Bug 27715: Use $dbh->quote_identifier to quote untrusted > input > >The sanitization using regex and \w class of characters might be >enough but given the vast number of unicode characters in \w and >possibility of in the future the database engines interpreting some of >those characters with special meaning it is better to wrap the column >identifier to quotes using $dbh->quote_identifier so it is only >interpreted as identifier and nothing else. > >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > C4/Utils/DataTables.pm | 15 ++++++++++++--- > t/db_dependent/Utils/Datatables.t | 4 ++-- > 2 files changed, 14 insertions(+), 5 deletions(-) > >diff --git a/C4/Utils/DataTables.pm b/C4/Utils/DataTables.pm >index b43849c008..23c7ebd405 100644 >--- a/C4/Utils/DataTables.pm >+++ b/C4/Utils/DataTables.pm >@@ -85,8 +85,8 @@ sub dt_build_orderby { > my $param = shift; > > my $i = 0; >- my $orderby; > my @orderbys; >+ my $dbh = C4::Context->dbh; > while(exists $param->{'iSortCol_'.$i}){ > my $iSortCol = $param->{'iSortCol_'.$i}; > my $sSortDir = $param->{'sSortDir_'.$i}; >@@ -105,9 +105,18 @@ sub dt_build_orderby { > return unless @orderbys; > > # Must be "branches.branchname asc", "borrowers.firstname desc", etc. >- @orderbys = grep { /^\w+\.\w+\s(asc|desc)$/ } @orderbys; >+ @orderbys = grep { /^\w+\.\w+ (asc|desc)$/ } @orderbys; >+ >+ my @sanitized_orderbys; >+ for my $orderby (@orderbys) { >+ my ($identifier, $direction) = split / /, $orderby, 2; >+ my ($table, $column) = split /\./, $identifier, 2; >+ my $sanitized_identifier = $dbh->quote_identifier(undef, $table, $column); >+ my $sanitized_direction = $direction eq 'asc' ? 'ASC' : 'DESC'; >+ push @sanitized_orderbys, "$sanitized_identifier $sanitized_direction"; >+ } > >- $orderby = " ORDER BY " . join(',', @orderbys) . " " if @orderbys; >+ my $orderby = " ORDER BY " . join(',', @sanitized_orderbys) . " " if @sanitized_orderbys; > return $orderby; > } > >diff --git a/t/db_dependent/Utils/Datatables.t b/t/db_dependent/Utils/Datatables.t >index 9f52ce3508..ae0c61bc4e 100644 >--- a/t/db_dependent/Utils/Datatables.t >+++ b/t/db_dependent/Utils/Datatables.t >@@ -47,7 +47,7 @@ subtest 'dt_build_orderby' => sub { > }; > > my $orderby = dt_build_orderby($dt_params); >- is( $orderby, " ORDER BY branches.branchname asc,borrowers.surname desc,borrowers.firstname desc ", 'ORDER BY has been correctly built' ); >+ is( $orderby, " ORDER BY `branches`.`branchname` ASC,`borrowers`.`surname` DESC,`borrowers`.`firstname` DESC ", 'ORDER BY has been correctly built' ); > > $dt_params = { > %$dt_params, >@@ -57,5 +57,5 @@ subtest 'dt_build_orderby' => sub { > }; > > $orderby = dt_build_orderby($dt_params); >- is( $orderby, " ORDER BY branches.branchname asc,borrowers.surname desc,borrowers.firstname desc ", 'ORDER BY has been correctly built, even with invalid stuff'); >+ is( $orderby, " ORDER BY `branches`.`branchname` ASC,`borrowers`.`surname` DESC,`borrowers`.`firstname` DESC ", 'ORDER BY has been correctly built, even with invalid stuff'); > }; >-- >2.20.1
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 27715
:
116992
|
116993
|
116994
|
116995
|
116998
|
117027
|
117028
|
117029
|
117030
|
117031
|
117032
|
117033
|
117034
|
117035
|
117036
|
117068
|
117069
|
117070
|
117071
|
117072
|
117073
|
117074
|
117076
|
117077
|
117078
|
117079
|
117080
|
117081
|
117082
|
117112
|
117113
|
117114
|
117115
|
117116
|
117117
|
117118