From 8affddc52db84d0da95b5460fbe8d2a1a1e34942 Mon Sep 17 00:00:00 2001
From: Jared Camins-Esakov <jcamins@cpbibliography.com>
Date: Sat, 7 Jul 2012 08:53:49 -0400
Subject: [PATCH] Bug 8268 follow-up: incorporate QA comments

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.

Signed-off-by: Robin Sheat <robin@catalyst.net.nz>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
---
 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 d8fbd7c1f9..0858059da9 100644
--- a/debian/templates/koha-conf-site.xml.in
+++ b/debian/templates/koha-conf-site.xml.in
@@ -263,7 +263,7 @@
  <intrahtdocs>/usr/share/koha/intranet/htdocs/intranet-tmpl</intrahtdocs>
  <includes>/usr/share/koha/intranet/htdocs/intranet-tmpl/prog/en/includes/</includes>
  <logdir>/var/log/koha/__KOHASITE__</logdir>
- <backupdir>/var/lib/koha/__KOHASITE__</backupdir>
+ <backupdir>/var/spool/koha/__KOHASITE__</backupdir>
  <!-- Enable the two following to allow superlibrarians to download
       database and configuration dumps (respectively) from the Export
       tool -->
diff --git a/misc/cronjobs/backup.sh b/misc/cronjobs/backup.sh
index 0806c6ca3b..fdd6a21455 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 3355c2af7f..d3ad16f57c 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) );
-- 
2.39.5