Database backup submission

fireartist has asked for the wisdom of the Perl Monks concerning the following question:

Greetings,
This is the first script I've written that deals with writing files to disk, so I'd like to submit this for your perusal, so that if I've done something unwise or left something amiss, you may guide me to clarity and greater understanding.

The script grabs a remote mysql database and backs it up to disk, first making a copy of the backup file.

The intention is that this will run as a daily cron job.
In the future I will probably expand it so that it compresses the file and emails it as well as saving to disk, and also email notification of errors, but at the moment that's not necessary.
I explain this because, yes, I can see it's a bit pointless printing error messages from a script that will be running unattended.

#!/usr/bin/perl -wT
use strict;

### These 2 files should be absolute references
my %file;
$file{1} = '/path/to/backup.sql';
$file{2} = '/path/to/backup.bck';
my %db;
$db{prog} = '/usr/bin/mysqldump';
$db{host} = 'mysql.host.com';
$db{user} = 'user';
$db{password} = 'pass';
$db{database} = 'database';

### END of configuration section

### Declare global variables
my $file1_data;

### Check the file paths for dangerous stuff
foreach (keys %file) {
    # Check it starts with a '/'
    if ($file{"$_"} !~ /^\//) {
    print "insecure file path";
    exit;
    }
    # Check it doesn't have any double dots
    if ($file{"$_"} =~ /\.\./) {
    print "insecure file path";
    exit;
    }
}

### Check the db vars
foreach (keys %db) {
    # Allowed chars are: A-Za-z0-9 _/.-
    unless ($db{"$_"} =~ /^[a-zA-Z0-9_\.\/-]+$/) {
    print "insecure database config";
    exit;
    }
}

### Any other security stuff
$ENV{'PATH'} = undef;

### Back up the file, if it exists
if ( open(FILE1, "< $file{1}") ) {
    open(FILE2, "> $file{2}");
    while (<FILE1>) {
    $file1_data .= $_;
    }
    close (FILE1);
    print FILE2 $file1_data;
    close (FILE2);
}

### Copy the database
system("$db{'prog'} --opt -h $db{'host'} -u $db{'user'} --password=$db
+{'password'} $db{'database'} > $file{'1'}");
exit;
[download]

Also, looking through the code, I've just remembered a problem with it that I don't understand.
Initially, the script was dying, complaining of a Taint error with the system call in the path for mysqldump.
So, I added the $ENV{'PATH'} = undef; line and now pass the full path for the mysqldump program.
Now it runs, but with a warning about the line where I undef $ENV('PATH').
Any ideas of what's going wrong there would be appreciated.

Comment on Database backup submission Download Code

Replies are listed 'Best First'.
Re: Database backup submission by jarich (Curate) on Apr 26, 2002 at 01:01 UTC
if I've done something unwise or left something amiss You're not checking the return values of both of your file opens. This is generally a bad thing, because reading from or writing to an un-opened filehandle yields undefined behaviour. Note that if your first open fails, you'll still attempt to do the database copy. Is that desireable? It also seems to me that you could make your file-copy section much more efficient if you considered the File::Copy module. For example: `### Back up the file, if it exists use File::Copy; if(-e $file{1}) # if the file exists { unless(copy($file{1}, $file{2})) { print "Failed to copy file: $!"; exit; # unless you want to do the copy anyway } } ### Copy the database` [download] Alternately if you like hand-rolled solutions, consider the following: `if(-e $file{1}) # if the file exists { open FILE1, "< $file{1} or die "Failed to open $file{1}". " for reading: $!"; local $/ = ""; # prepare to read the whole # file in at once my $filecontents = <FILE1>; close FILE1; open FILE2, "> $file{2} or die "Failed to open $file{2}". " for writing: $!"; print FILE2 $filecontents; close FILE2; }` [download] Hope that helps. :) (Oh, and note that $file1_data, or $filecontents in my example, need not be global at all.) jarich	[reply] [d/l] [select]
Re: Re: Database backup submission by fireartist (Chaplain) on Apr 30, 2002 at 10:58 UTC
You're not checking the return values of both of your file opens. Thanks, I shouldn't have missed that Thanks also for pointing out that I could use File::Copy, I certainly need practise getting into the mode of thinking "is there a module that could do this?"	[reply]
Re: Database backup submission by samgold (Scribe) on Apr 26, 2002 at 01:16 UTC
I don't know much about MySql but you should, if you haven't already, read the documentation about backups to make sure that you are backing up everything that you need to. I am an Oracle DBA and a common mistake in backing up databases it to not back up all of the necessary files. Check out Oracle DB & Server Backup it may give you some ideas. If you have any questions feel free to send me an email. Sam	[reply]
Re: Database backup submission by jarich (Curate) on Apr 29, 2002 at 01:38 UTC
`### Any other security stuff $ENV{'PATH'} = undef;` [download] Also, looking through the code, I've just remembered a problem with it that I don't understand. You're getting the warning with the $ENV{PATH} set to undef because $ENV{PATH} is checked when system calls are made. If you want to get rid of this error set $ENV{PATH} to "" instead. The reason $ENV{PATH} failed taint checking altogether is that the assumption is that any user running your script can edit their $ENV{PATH} to whatever they like. Hence relying on $ENV{PATH} to be sensible is like assuming any other user data is sensible. You could also have just untainted $ENV{PATH}, but setting the full path is the much wiser option. jarich	[reply] [d/l]