I have a script which runs a database query and redirects the result to a csv file and after using sftp to upload it, the file is removed like this:

FileName=` echo "report_$StartDate:$StopDate.csv" | sed 's/\ /_/g'`

$DatabaseCommand "$Query" -f CSV | sed 's/"//g' > "$ReportDir/$FileName"

set timeout 3600
spawn sftp $USER@$HOST
expect "password:"
send "$PASSWORD\r"
expect "sftp>"
send "put $ReportDir/$FileName\r"
expect "sftp>"
send "bye\r"

rm -rf $ReportDir/$FileName

I know tha last line should be changed to rm -f, but I'm wondering is there any way that bash could mix up in the FileName line and be able to run the following command instead:

rm -rf $ReportDir/
  • 2
    FileName turns out to be empty, or starts with spaces so because of your lack of quoting, gets split up? – muru Dec 19 '17 at 7:19
  • @muru I've run this many times without any problem but I want to make sure it is safe and not able to remove anything. – Amin Alaee Dec 19 '17 at 7:24
  • What happens the one time you do have a problem and FileName becomes something like report_*:*.csv and you end up deleting all those files in the directory? – muru Dec 19 '17 at 7:38
  • @muru It'd delete all files starting with report_. But is there a way the script can somehow skip FileName (error or anything) and run rm -rf $ReportDir/ – Amin Alaee Dec 19 '17 at 7:43

With a command like that, you may want to guard against the variables being empty, regardless of how that could happen:

$ cat foo.sh
FileNam=$(echo ...) # oops, a typo
# ... 
rm -rf -- "${ReportDir:?}/${FileName:?}"

$ bash -x foo.sh
+ ReportDir=/somepath
++ echo ...
+ FileNam=...
foo.sh: line 5: FileName: parameter null or not set

The ${parameter:?message} expansion exits with an error if parameter is empty or unset. The default message is usually just fine, so I didn't give one in the code above.

Alternatively, just check manually, but this is verbose and still prone to typos:

if [ -z "$FileName" ] ; then
    echo "FileName is empty!" >&2
    exit 1
rm -rf -- "$ReportDir/$FileName"

Since StartDate, StopDate and ReportDir are not set in the script you have shown us, I'm going to assume this is not the entire script. So:

$ cat foo.sh
# commands from what you have shown, omitting `expect` and db command
FileName=` echo "report_$StartDate:$StopDate.csv" | sed 's/\ /_/g'`

rm -rf $ReportDir/$FileName
$ bash -ux foo.sh
+ ReportDir=foo
foo.sh: line 2: StartDate: unbound variable
++ sed 's/\ /_/g'
+ FileName=
+ rm -rf foo/

Clearly it expanded FileName to nothing and ran rm -rf foo/.

  • 1
    well, of course that only happens if you explicitly set -u, otherwise FileName would contain at least report_: – ilkkachu Dec 19 '17 at 8:37
  • 1
    @ilkkachu or if PATH was broken in a way that left rm available but not sed – muru Dec 19 '17 at 8:59

Consider using sshpass instead.

SSHPass is a tiny utility, which allows you to provide the ssh password without using the prompt. This will very helpful for scripting. SSHPass is not good to use in multi-user environment. If you use SSHPass on your development machine, it don't do anything evil.

  • That's one option, but not really what the question here is about, no? – ilkkachu Dec 19 '17 at 8:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.