4
\$\begingroup\$

How secure is my script?

#!/usr/bin/env bash

echo "Input database name"
read databaseName
echo "The database name is set to $databaseName"

echo "Input database host"
read databaseHost
echo "The database host is set to $databaseHost"

echo "Input database user"
read user
echo "The database user is set to $user"

stty -echo
echo "Input database user password"
read password
stty echo

echo "Input url to search for"
read searchUrl
echo "The search url is set to $searchUrl"

echo "Input url to replace with the searched url"
read replaceUrl
echo "The replace url is set to $replaceUrl"

mysql --user="$user" --password="$password" --host="$databaseHost" --database="$databaseName" -e "UPDATE messages SET message = REPLACE(message, '$searchUrl', '$replaceUrl')"

Is it bad practise to get the user his password like this? Or is there anything I can improve?

\$\endgroup\$
6
\$\begingroup\$

You can make Bash's read built emit a prompt by using its -p option:

read -p "Database name: " databaseName

and you can make it turn off echoing with -s:

read -s -p "Database password for $user: " password

Avoid writing "$password" into the mysql command line like that - as the man page says:

Specifying a password on the command line should be considered insecure. See Section 6.1.2.1, “End-User Guidelines for Password Security”. You can use an option file to avoid giving the password on the command line.

It also says

If you omit the password value following the --password or -p option on the command line, mysql prompts for one.

So I'd recommend not reading the password in this script, but passing --password to tell mysql to do its own prompting.

The other thing that's dangerous is using user input to construct the SQL query string - even if your users are trusted (and you know they have permission to connect and execute this command), it's worth validating the strings to prevent accidents. An alternative could be to use Bash to quote the arguments:

command=$(printf 'UPDATE messages SET message = REPLACE(message, %q, %q') \
                 "$searchUrl", "$replaceUrl")

mysql --user="$user" --password \
   --host="$databaseHost" --database="$databaseName" \
   -e "$command"

It may be worth writing the script in a different language (or passing these variables to a sub-script in the different language), to enable the use of prepared statements. I thought there might be a way to define variables for the mysql command like you can for Awk, and then use them in statements, but I couldn't find any provision for that, unfortunately.

An enhancement I would make is to ask for confirmation after reading all the interactive inputs. Right now, if I make a mistake in replaceUrl (e.g. I manage to press \ and Enter together), then I don't get a chance to interrupt the update. At the very least sleep 2 && mysql so I can Control+C before it's too late.

\$\endgroup\$
  • \$\begingroup\$ I've tried to use the -p and -s flags but they give me an error :s That's why I didn't make use of it.. \$\endgroup\$ – Frank Groot Apr 16 '18 at 13:58
  • 1
    \$\begingroup\$ @TobySpeight You’re right about SQL Injection. I know all about Bobby Tables. My error was to forget that -e executes everything in site, so, while the UPDATE statement was save, the EXECUTE was not. I’m beginning to think that it’s not possible using a BASH script. I have deleted my answer. Thanks. \$\endgroup\$ – Manngo Apr 16 '18 at 23:59
  • \$\begingroup\$ @TobySpeight GNU bash, version 4.4.12(1)-release (x86_64-pc-linux-gnu) \$\endgroup\$ – Frank Groot Apr 17 '18 at 8:21
  • \$\begingroup\$ read -p 'Foo: ' -s foo works for me with GNU bash, version 4.2.8(1)-release (i686-pc-linux-gnu), so I don't understand why it doesn't for you. \$\endgroup\$ – Toby Speight Apr 17 '18 at 9:43

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.