[Koha-bugs] [Bug 8268] Koha should offer way to backup entire db

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8268

--- Comment #12 from Frère Sébastien Marie <semarie-koha at latrappe.fr> ---
(En réponse au commentaire 11)
>
> Marking passed QA, I'll delay pushing, I'll wait for some feedback from
> semarie, just in case

The code seems globally ok: just 4 remarks.

Some security remarks (in tools/export.pl):
 1. a problem (common in koha): use of user-input in warn. An attacker can
forge entries in log file (see CWE-93) (resume: if variable contains CRLF, the
log file too... and the next line is forged: so who write in log ? koha or
attacker ?)

> warn "A suspicious attempt was made to download the db at '$filename' by ..."


 2. about the regex for $filename verification
> return unless ( ... && not $filename =~ m#(^\.\.|/)# );

The regex could be translated as: $filename should not:
 - start with '..'
OR
 - contains '/'

I don't understand the purpose of "^..", if '/' is forbidden; as you can't
escape from directory without '/'.

So I think m#/# is suffisent (please correct me if not).



Some generals remarks:
 3. the directory for backup is not consitent in differents files:

in debian/templates/koha-conf-site.xml.in, the directory use for backupdir is:
> <backupdir>/var/lib/koha/__KOHASITE__</backupdir>

in Makefile.PL, the directory seems to be in /var/spool :
> './skel/var/spool/koha'       => { target => 'BACKUP_DIR', trimdir => -1 },

in debian/scripts/koha-dump:
> [ -z "$backupdir" ] && backupdir="/var/spool/koha/$name"


 4. in misc/cronjobs/backup.sh

The lasts lines are for mail admin if ok, or mail admin if not ok.
> [ -f $KOHA_BACKUP] && echo "$KOHA_BACKUP was successfully created." | mail kohaadmin -s $KOHA_BACKUP ||
> echo "$KOHA_BACKUP was NOT successfully created." | mail kohaadmin -s $KOHA_BACKUP

First, there are *two* lines. I haven't test it, but it should be an error as
"cmd_a && cmd_b ||" is not a valid shell expression. Should be one-line "cmd_a
&& cmd_b || cmd_c"

Secondly, I'm not sure that the behaviour is correct.
"cmd_a && cmd_b || cmd_c" could be expanded as:

if cmd_a ; then
  if ! cmd_b ; then
    cmd_c
  fi
else
  cmd_c
fi

so if cmd_b failed (echo "backup ok" | mail admin), the cmd_c (echo "backup
err" | mail admin) will be run (and expected to failed too, as the first try).

Proposed correction:
> 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

-- 
You are receiving this mail because:
You are watching all bug changes.