[ forwarded from debian-devel to have this included in the ITP ] From: Jakub Wilk <jw...@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-de...@lists.debian.org Date: Wed, 24 Feb 2016 23:26:11 +0100
* Brian May <b...@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