Security Bugfix: Bug 1953 Adding Placeholders to SQL To Avoid Potential Injection Attacks

This patch addresses both security issues mentioned in the summary of the report
submitted by Frère Sébastien Marie included below.

---------------------------
The problem is here: 'C4/AuthoritiesMarc.pm' in the function 'DelAuthority':
The argument $authid is included directly (not via statement) in the SQL.

For the exploit of this problem, you can use 'authorities/authorities-home.pl'
with authid on the URL and op=delete (something like
"authorities/authorities-home.pl?op=delete&authid=xxx").

This should successfully call DelAuthority, without authentification...
(DelAuthority is call BEFORE get_template_and_user, so before authentification
[This should be an issue also...]).

Please note that the problem isn't only that anyone can delete an authority of
this choose, it is more general: with "authid=1%20or%1=1" (after inclusion sql
will be like: "delete from auth_header where authid=1 or 1=1") you delete all
authorities ; with "authid=1;delete%20from%xxx" it is "delete from auth_header
where authid=1;delete from xxx" and so delete what you want...

SQL-INJECTION is very permissive: you can redirect the output in a file (with
some MySQL function), so write thea file of you choose in the server, in order
to create a backdoor, and compromise the server.

Signed-off-by: Frère Sébastien Marie <semarie-koha@latrappe.fr>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
This commit is contained in:
Chris Nighswonger 2011-02-24 09:57:11 -05:00 committed by Chris Cormack
parent 2a3f7c1417
commit b0f60221f4
2 changed files with 3 additions and 6 deletions

View file

@ -719,8 +719,8 @@ sub DelAuthority {
my $dbh=C4::Context->dbh;
ModZebra($authid,"recordDelete","authorityserver",GetAuthority($authid),undef);
$dbh->do("delete from auth_header where authid=$authid") ;
my $sth = prepare("DELETE FROM auth_header WHERE authid=?");
$sth->execute($authid);
}
sub ModAuthority {

View file

@ -147,9 +147,6 @@ if ( $op eq "do_search" ) {
}
elsif ( $op eq "delete" ) {
&DelAuthority( $authid, 1 );
( $template, $loggedinuser, $cookie ) = get_template_and_user(
{
template_name => "authorities/authorities-home.tmpl",
@ -160,7 +157,7 @@ elsif ( $op eq "delete" ) {
debug => 1,
}
);
&DelAuthority( $authid, 1 );
}
else {
( $template, $loggedinuser, $cookie ) = get_template_and_user(