Bug 17717: Add a --chdir option switch for koha-foreach

Until Perl 5.26, the current directory is added to @INC when running a
Perl script [1]. Having the current directory in @INC means it can be
tried to be traversed when performing a lib lookup. Since version 5.18,
Perl dies when it finds an unreadable directory (permissions) in @INC
that needs to be traversed. This behaviour won't change because Perl
devs consider it an enhancement to security. [2]

Because of this, we need to make sure our scripts are ran **from** a
directory in which they have read permissions.

Ths patch adds a --chdir option switch to the **koha-foreach** wrapper
script, that makes the inner shells/scripts to be ran within the Koha
instance's user home directory.

The change is trivial and should be QAed easily. I tested this on a prod
server:

- Create a /tmp/test.pl file containing:

use Modern::Perl;

use Cwd;
my $dir = getcwd;

warn $dir;

1;

A) then create a cronjob entry to run it using koha-foreach:
(in /etc/cron.d/test):
1/* * * * * root koha-foreach perl /tmp/test.pl
- Once I noticed the cronjob ran, I used mutt to read the emails in the
root user.
=> FAIL:
...
Subject: Cron <root@koha> koha-foreach --enabled perl /tmp/test.pl

"/root"
"/root"
"/root"
"/root"
"/root"
...

B) I then used the patched koha-foreach with different results:
=> SUCCESS:
...
Subject: Cron <root@koha> /root/koha-foreach --chdir --enabled perl /tmp/test.pl

"/var/lib/koha/acaderc"
"/var/lib/koha/agro"
"/var/lib/koha/anc"
"/var/lib/koha/arico"
"/var/lib/koha/artes"
...

So this patch's approach works. But...

C) master's koha-foreach seems to work just the same... I think it is
because of my previous attempt to fix this by using sudo in koha-shell.
So I think environmental conditions affect the behaviour (which shell is
configured for cron, sudo configuration, etc).

====

In conclusion, I think we should go ahead with this patch as it will solve
peoples issues, and it is a right solution (option #5 on the list) to
this Perl behaviour change. It doesn't cover other commands, but
followup patches could do.

I avoided /tmp as it is writable by any user... so it is an easy path
for both exploiting by replacing some lib, and also because the
existence of an unreadable dir that the interpreter could try to
traverse (unreadable /tmp/Authen or /tmp/Koha will trigger the same
error, and I assume people know what they are putting on the instance's
dir, at least it will be easier to track).

A followup patch takes care of making the cronjobs use --chdir when
calling koha-foreach

[1] https://lists.debian.org/debian-devel-announce/2016/08/msg00013.html
[2] https://rt.perl.org/Public/Bug/Display.html?id=123795

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(cherry picked from commit 3db7e1a63a517ad03a5426504ef4010beae0c8bf)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

diff --git a/debian/docs/koha-foreach.xml b/debian/docs/koha-foreach.xml
index 4e84463..ccf1ab5 100644 (file)
--- a/debian/docs/koha-foreach.xml
+++ b/debian/docs/koha-foreach.xml
@@ -23,7 +23,7 @@
 
   <refsynopsisdiv>
     <cmdsynopsis>
-      <command>koha-foreach</command> <arg><option>--enabled</option></arg> <arg><option>--email</option>|<option>--noemail</option></arg> <arg><option>command</option></arg>
+      <command>koha-foreach</command> <arg><option>--chdir</option></arg> <arg><option>--enabled</option></arg> <arg><option>--email</option>|<option>--noemail</option></arg> <arg><option>command</option></arg>
     </cmdsynopsis>
   </refsynopsisdiv>
 
@@ -31,7 +31,20 @@
   <para>Run a command for each Koha instance. Takes the same arguments as koha-list.</para>
   <para>The string "__instancename__" is replaced in the argument list with the name of the Koha instance in each iteration.</para>
   </refsect1>
-  
+
+  <refsect1>
+    <title>Options</title>
+      <variablelist>
+      <varlistentry>
+        <term><option>--chdir</option></term>
+        <listitem>
+          <para>This option makes the command jump into the instance's home directory before running the required command. This prevents
+            directory traversal permissions issues.</para>
+        </listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+
   <refsect1><title>See also</title>
   <simplelist type="inline">
     <member><command>koha-create-dirs(8)</command></member>

diff --git a/debian/scripts/koha-foreach b/debian/scripts/koha-foreach
index 50f96c7..47f92c5 100755 (executable)
--- a/debian/scripts/koha-foreach
+++ b/debian/scripts/koha-foreach
@@ -28,10 +28,14 @@ else
     exit 1
 fi
 
+chdir="no"
+starting_dir=$(pwd)
+
 listopts=""
 while [ ! -z "$1" ]
 do
     case "$1" in
+        --chdir) chdir="yes";;
         --email) listopts="$listopts --email";;
       --noemail) listopts="$listopts --noemail";;
       --enabled) listopts="$listopts --enabled";;
@@ -53,6 +57,13 @@ do
     cmd=`echo "$@" | sed -e s/__instancename__/${name}/g`
 
     if [ "${cmd}" != "" ]; then
+
+        # Change to the instance's home dir if required
+        [ "chdir" != "no" ] && eval cd ~$name"-koha"
+
         koha-shell ${name} -c "${cmd}"
+
+        # Go back to the original dir if required
+        [ "chdir" != "no" ] && cd $starting_dir
     fi
 done