From: Chris Nighswonger <[email protected]> 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 <[email protected]> Signed-off-by: Chris Cormack <[email protected]> Signed-off-by: Galen Charlton <[email protected]> --- C4/AuthoritiesMarc.pm | 4 ++-- authorities/authorities-home.pl | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/C4/AuthoritiesMarc.pm b/C4/AuthoritiesMarc.pm index 7d9a2b4..6323f7d 100644 --- a/C4/AuthoritiesMarc.pm +++ b/C4/AuthoritiesMarc.pm @@ -634,8 +634,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 { diff --git a/authorities/authorities-home.pl b/authorities/authorities-home.pl index 1eb58b0..919be5b 100755 --- a/authorities/authorities-home.pl +++ b/authorities/authorities-home.pl @@ -136,9 +136,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", @@ -152,6 +149,7 @@ elsif ( $op eq "delete" ) { # $template->param("statements" => \@statements, # "nbstatements" => $nbstatements); + &DelAuthority( $authid, 1 ); } else { ( $template, $loggedinuser, $cookie ) = get_template_and_user( -- 1.7.2.3 _______________________________________________ Koha-patches mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-patches website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
