Two security issues with the update script

[orlitzky]

Hi, I'm in the process of adding the urlhaus signatures to my system and I noticed two security issues with the update script:

The first is that you should never use a fixed path under /tmp because it can almost always be exploited. Instead, something like TMPDIR=$(mktemp --directory) can be used instead. The random name that mktemp creates makes it "impossible" for an attacker to predict the name before you use it. I'm not sure if the lockfile path is at risk, but I would play it safe and use mktemp for that, too. The use of /tmp/urlhaus is exploitable because anyone on the machine can create that directory (by winning an easy race condition) and then manipulate the contents of a directory that they own.

The second is that calling chown on a file you don't control is unsafe. The chown utility follows hard and symlinks by default, so if the clamav user replaces the new database with a symlink to /etc/passwd (he can do this because he owns the parent directory $CLAMDIR), then the script will give him ownership of that file. To fix this, I would suggest just not doing it. Tell people to run the cron job as the clamav user and not as root to begin with. That's safer all around.

[chrcoluk]

your TMPDIR variable line errors out for me on bash script added as variable although the standalone command works fine on a shell.

'mktemp: illegal option -- -'

Otherwise I think its a great idea.