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.

• glenn jackman: Why don't you delete it?
• Andrea Corbellini: I guess you are using chmod 000 (instead of rm) to allow later inspection. If that's the case, then I'd move the file to another directory: in some situations, files may be read even if they are -r.
• tripleee: You need to put any variable containing fileeor directory names in double quotes. Otherwise, names containing spaces, shell metacharacters etc will not work.
• tripleee: Your script will not work correctly on file names containing colons, either.
• Anatol: Hi triplee thanks for your reply means else chmod 000 "$STR" should do it?

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"
  

• janos: The way he loops is safe for directories containing spaces, your suggestion is simple at the expense of safety sacrificed
• Håken Lid: What does read -d : str rest <<< "$file" do?