From a3578f94d4e484ea4d81d6d3b12bac5abf3ac885 Mon Sep 17 00:00:00 2001 From: Jared Camins-Esakov Date: Sat, 7 Jul 2012 08:53:49 -0400 Subject: [PATCH] Bug 8268 follow-up: incorporate QA comments Content-Type: text/plain; charset="UTF-8" Fixes the following things: 1. Sanitizes log output to prevent an attacker from using a specially crafted POST to add extra lines to the log 2. Simplify a regular expression since "..file" cannot be used to escape the current directory 3. Makes sure directories are consistent 4. Correct logic issues in misc/cronjobs/backup.sh Thanks to Frere Sebastien Marie for catching these issues. --- debian/templates/koha-conf-site.xml.in | 2 +- misc/cronjobs/backup.sh | 6 +++++- tools/export.pl | 11 ++++++++--- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/debian/templates/koha-conf-site.xml.in b/debian/templates/koha-conf-site.xml.in index d8fbd7c..0858059 100644 --- a/debian/templates/koha-conf-site.xml.in +++ b/debian/templates/koha-conf-site.xml.in @@ -263,7 +263,7 @@ /usr/share/koha/intranet/htdocs/intranet-tmpl /usr/share/koha/intranet/htdocs/intranet-tmpl/prog/en/includes/ /var/log/koha/__KOHASITE__ - /var/lib/koha/__KOHASITE__ + /var/spool/koha/__KOHASITE__ diff --git a/misc/cronjobs/backup.sh b/misc/cronjobs/backup.sh index 0806c6c..fdd6a21 100755 --- a/misc/cronjobs/backup.sh +++ b/misc/cronjobs/backup.sh @@ -13,7 +13,11 @@ mysqldump --single-transaction -u koha -ppassword koha | gzip -9 > $KOHA_BACKUP # Makes the compressed dump file property of the kohaadmin user. # Make sure that you replace kohaadmin with a real user. -[ -f $KOHA_BACKUP] && echo "$KOHA_BACKUP was successfully created." | mail kohaadmin -s $KOHA_BACKUP || +if [ -f $KOHA_BACKUP ] ; then +echo "$KOHA_BACKUP was successfully created." | mail kohaadmin -s $KOHA_BACKUP +else echo "$KOHA_BACKUP was NOT successfully created." | mail kohaadmin -s $KOHA_BACKUP +fi + # Notifies kohaadmin of (un)successful backup creation # EOF diff --git a/tools/export.pl b/tools/export.pl index 3355c2a..d3ad16f 100755 --- a/tools/export.pl +++ b/tools/export.pl @@ -31,6 +31,7 @@ use Data::Dumper; my $query = new CGI; my $op=$query->param("op") || ''; my $filename=$query->param("filename"); +$filename =~ s/(\r|\n)//; my $dbh=C4::Context->dbh; my $marcflavour = C4::Context->preference("marcflavour"); @@ -179,7 +180,9 @@ if ($op eq "export") { $successful_export = download_backup( { directory => "$backupdir", extension => 'sql', filename => "$filename" } ) } unless ( $successful_export ) { - warn "A suspicious attempt was made to download the db at '$filename' by someone at " . $query->remote_host() . "\n"; + my $remotehost = $query->remote_host(); + $remotehost =~ s/(\n|\r)//; + warn "A suspicious attempt was made to download the db at '$filename' by someone at " . $remotehost . "\n"; } exit; } @@ -189,7 +192,9 @@ if ($op eq "export") { $successful_export = download_backup( { directory => "$backupdir", extension => 'tar', filename => "$filename" } ) } unless ( $successful_export ) { - warn "A suspicious attempt was made to download the configuration at '$filename' by someone at " . $query->remote_host() . "\n"; + my $remotehost = $query->remote_host(); + $remotehost =~ s/(\n|\r)//; + warn "A suspicious attempt was made to download the configuration at '$filename' by someone at " . $remotehost . "\n"; } exit; } @@ -340,7 +345,7 @@ sub download_backup { my $filename = $args->{filename}; return unless ( $directory && -d $directory ); - return unless ( $filename =~ m/$extension(\.(gz|bz2|xz))?$/ && not $filename =~ m#(^\.\.|/)# ); + return unless ( $filename =~ m/$extension(\.(gz|bz2|xz))?$/ && not $filename =~ m#|# ); $filename = "$directory/$filename"; return unless ( -f $filename && -r $filename ); return unless ( open(my $dump, '<', $filename) ); -- 1.7.2.5