Grep for pattern recursive and disable file

Question

On a shared host, I'd like to setup a cron which scans folders recursively for some base64 malware strings. Therefore, I've written the following script:

#!/bin/bash

if [ $# -ne 1 ]; then
    echo $0: usage: ./findone folder_to_start_with
    exit 1
fi

folder=$1

IFS=$'\n'
searchfiles=($(grep -r -F -n -f malware-strings.dat $folder))

for (( i=0; i<${#searchfiles[@]}; i++ ));
        do
                STR=$(echo ${searchfiles[i]} | awk -F':' '{print $1}')
                    if [ -z "$STR" ]; 
                        then true; 
                        else chmod 000 $STR;
                    fi            
        done

## Do something else like mail results etc.
printf '%s\n' "${searchfiles[@]}"

My locals test are doing what I expect. If a string pattern from "malware-string.dat" is found the file permission is changed to 000. Before scanning the production sites, I wanted to ask for a code review as I'm new to Bash and do not want to mess things up. Also, your judgement of disabling the file with chmod is enough would help, or if it is advisable to move the file outside of the www directory.

Answer 1

Your input checking is not very strict. Any argument could pass that check, even an empty string, but the script needs specifically a valid path to a folder. So why not check for that:

if [ ! -d "$1" ]; then
    echo $0: usage: ./findone folder_to_start_with
    exit 1
fi

The grep command will output the matched lines, with the line number. But you don't need those, only the filename. And there could be multiple matches per file. It would be better to use the -l flag, and that way you won't need the awk at all, you will get only the file names, ready to use.

This condition is very awkward:

if [ -z "$STR" ]; 
    then true; 
    else chmod 000 $STR;
fi

Instead of an empty branch that does nothing, you could have flipped the condition by simply dropping the -z, and the unused branch with it, resulting in the simpler:

if [ "$STR" ]; 
    then chmod 000 $STR
fi       

(In any case, when you rewrite the grep to use -l, this condition will not be needed, so this part will be simply gone.)

As you guessed, in addition to chmod it would be better to move the file out of the vulnerable directory, to a safer place. One reason is to make it less vulnerable, another is that these files buried deep within your web directories might go unnoticed easier. I'd also suggest adding email alerts after detecting such malware.

Answer 2

• Bash idiom for looping over searchfiles is

  for file in $searchfiles; do
      process "$file"
  

• Invoking awk just to print a field looks like an overkill. Consider

  read -d : str rest <<< "$file"