Bugzilla – Attachment 72372 Details for
Bug 17717
Fix broken cronjobs due to permissions of the current directory
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 17717: Add a --chdir option switch for koha-foreach
Bug-17717-Add-a---chdir-option-switch-for-koha-for.patch (text/plain), 5.33 KB, created by
Kyle M Hall (khall)
on 2018-03-02 15:50:15 UTC
(
hide
)
Description:
Bug 17717: Add a --chdir option switch for koha-foreach
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2018-03-02 15:50:15 UTC
Size:
5.33 KB
patch
obsolete
>From c4dc8e61df696de151ccbfee713ff171af401690 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@gmail.com> >Date: Fri, 2 Mar 2018 11:17:40 -0300 >Subject: [PATCH] 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> >--- > debian/docs/koha-foreach.xml | 17 +++++++++++++++-- > debian/scripts/koha-foreach | 11 +++++++++++ > 2 files changed, 26 insertions(+), 2 deletions(-) > >diff --git a/debian/docs/koha-foreach.xml b/debian/docs/koha-foreach.xml >index 4e84463..ccf1ab5 100644 >--- 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 >--- 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 >-- >2.10.2
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 17717
:
57947
|
63676
|
63677
|
72368
|
72369
|
72372
|
72373
|
72411
|
72412
|
72413