4
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Why don't you delete it? \$\endgroup\$ – glenn jackman Feb 2 '16 at 14:52
  • \$\begingroup\$ 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. \$\endgroup\$ – Andrea Corbellini Feb 2 '16 at 15:13
  • \$\begingroup\$ You need to put any variable containing fileeor directory names in double quotes. Otherwise, names containing spaces, shell metacharacters etc will not work. \$\endgroup\$ – tripleee Feb 2 '16 at 15:28
  • \$\begingroup\$ Your script will not work correctly on file names containing colons, either. \$\endgroup\$ – tripleee Feb 2 '16 at 15:30
  • \$\begingroup\$ Hi triplee thanks for your reply means else chmod 000 "$STR" should do it? \$\endgroup\$ – t Book Feb 2 '16 at 16:17
2
\$\begingroup\$

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.

\$\endgroup\$
2
\$\begingroup\$
  • 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"
    
\$\endgroup\$
  • \$\begingroup\$ The way he loops is safe for directories containing spaces, your suggestion is simple at the expense of safety sacrificed \$\endgroup\$ – janos Feb 3 '16 at 17:30
  • \$\begingroup\$ What does read -d : str rest <<< "$file" do? \$\endgroup\$ – Håken Lid Feb 4 '16 at 21:06

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.