Bug#815675: ITP: ftpbackup -- Script to backups your data from a Debian system to a ftp space

[Nikolaus Rath <Nikolaus@rath.org>]

[ forwarded from debian-devel to have this included in the ITP ]

From: Jakub Wilk <jwilk@debian.org>
Subject: Re: Bug#815675: ITP: ftpbackup -- Script to backups your data from a Debian system to a ftp space
Newsgroups: gmane.linux.debian.devel.general
To: debian-devel@lists.debian.org
Date: Wed, 24 Feb 2016 23:26:11 +0100

* Brian May <bam@debian.org>, 2016-02-25, 08:14:
>I haven't seen the code myself, however one of the comments was:
>
>  "just having whitespace in the destination directory will lead to a
>  crash, set -e is not used, and errors are redirected to /dev/null"
>
>This sounds to me like a recipe for security problems.

I wouldn't worry about whitespace in destination directory. If the
attacker can control were backups go, we have a bigger problem...

But the bug density of this code is astounding:

>ARCHIVE=$BACKUPHOME/$SERVER-backup-`date +%d-%m-%Y`.tar.gz

Eww, little-endian dates.
https://xkcd.com/1179/

># create BACKUPHOME if not exists
>mkdir -p $BACKUPHOME

No umask set anywhere in this script, so in default setup the
directory (and later, the backup files) will be created readable to
anyone.

>dpkg --get-selections| awk -F' ' '{print $1}' > $PKGLIST
>RETVAL=$?
>if [[ $RETVAL != 0 ]]; then
>        echo "Issue while performing dpkg --get-selections of
> $SERVER" | mail -s "Issue while performing dpkg get selections of
> $SERVER" $ADMINEMAIL

Contrary to what the error message suggests, this catches only errors
from awk, not from dpkg.

>tar --preserve-permissions -z -c -f $ARCHIVE  \
>        --exclude=/var/lib/mysql/data \
>        --exclude=$BACKUPHOME/$SERVER-backup* \
>        --exclude=/var/log \
>        --exclude=/var/cache/apt/archives \
>        $EXCLUDES \
>        /etc /var /home /opt /usr/local/bin  > /dev/null 2>&1

What about /srv? Errors are hidden and ignored.

># remove old archive on the FTP
> lftp -e "set ftp:ssl-allow no; 

Not only this program lets the backups be sent over unencrypted
channel, but it even disables opportunistic TLS.

>rm -f $SERVER-backup-`date -d "-$RETENTION day" +%d-%m-%Y`.tar.bz2;exit" -u $FTPUSER,$FTPPASS $FTPSERVER > /dev/null 2>&1

This removal feature seems to work correctly only if you run backups
every day, and never close to midnight.

Wait, no, it doesn't work at all: the script creates .tar.gz, but then
it tries to delete .tar.bz2.

[Gratitude for the review would be best expressed by requesting
removal of this package from the archive.]

-- 
Jakub Wilk