Re: File preprocessor: suspected bug found and solved

["Hui Cao (huica)" <huica () cisco com>]

Hi Pablo,

Thanks for providing the patch. Currently, file agent does not log file type preprocessor event if file 
capture/signature is enabled. You can always use file_type keyword to generate a detection event.

Best,
Hui.

From: Pablo Cantos Polaino <pcantos () redborder org<mailto:pcantos () redborder org>>
Date: Thursday, May 21, 2015 at 10:11 AM
To: "snort-devel () lists sourceforge net<mailto:snort-devel () lists sourceforge net>" <snort-devel () lists 
sourceforge net<mailto:snort-devel () lists sourceforge net>>
Subject: [Snort-devel] File preprocessor: suspected bug found and solved

Hello all,

Some weeks ago I found a suspected bug related to the File preprocessor (follow the link: 
http://sourceforge.net/p/snort/mailman/message/33605425/):

snort/src/dynamic-preprocessors/file/file_agent.c:601-614
/*
 * File type callback when file type is identified
 *
 * For file capture or file signature, FILE_VERDICT_PENDING must be returned
 */
static File_Verdict file_agent_type_callback(void* p, void* ssnptr,
        uint32_t file_type_id, bool upload, uint32_t file_id)
{
    file_inspect_stats.file_types_total++;
    if (file_signature_enabled || file_capture_enabled)
        return FILE_VERDICT_UNKNOWN;
    else
        return FILE_VERDICT_LOG;
}
You can read on the description that FILE_VERDICT_PENDING must be returned when file capture OR file signature is 
enabled, but what really the code does is to return FILE_VERDICT_UNKNOWN when capture or signature are enabled.

This code is returning a FILE_VERDICT_UNKNOWN when signature or capture are enabled. The UNKNOWN verdict is not taken 
into account in the next code, as well as the PENDING verdict that the previous code refers to:

snort/src/file-process/file_service.c:734-784
        if (context->file_type_id != SNORT_FILE_TYPE_CONTINUE)
        {
            if (file_type_cb)
            {
                verdict = file_type_cb(p, ssnptr, context->file_type_id,
                        context->upload, context->file_id);
                file_stats.verdicts_type[verdict]++;
            }
            context->file_type_enabled = false;
            file_stats.files_processed[context->file_type_id][context->upload]++;
#ifdef TARGET_BASED
            file_stats.files_by_proto[context->app_id]++;
#endif
        }
        if (verdict == FILE_VERDICT_LOG )
        {
            file_eventq_add(GENERATOR_FILE_TYPE, context->file_type_id,
                    file_type_name(context->file_config, context->file_type_id),
                    RULE_TYPE__ALERT);
            context->file_signature_enabled = false;
            pkt->packet_flags |= PKT_FILE_EVENT_SET;
        }
        else if (verdict == FILE_VERDICT_BLOCK)
        {
            file_eventq_add(GENERATOR_FILE_TYPE, context->file_type_id,
                    file_type_name(context->file_config, context->file_type_id),
                    RULE_TYPE__DROP);
            updateFileSize(context, data_size, position);
            context->file_signature_enabled = false;
            add_file_to_block(p, verdict, context->file_type_id, NULL);
            return 1;
        }
        else if (verdict == FILE_VERDICT_REJECT)
        {
            file_eventq_add(GENERATOR_FILE_TYPE, context->file_type_id,
                    file_type_name(context->file_config, context->file_type_id),
                    RULE_TYPE__REJECT);
            updateFileSize(context, data_size, position);
            context->file_signature_enabled = false;
            add_file_to_block(p, verdict, context->file_type_id, NULL);
            return 1;
        }
        else if (verdict == FILE_VERDICT_STOP)
        {
            context->file_signature_enabled = false;
        }
        else if (verdict == FILE_VERDICT_STOP_CAPTURE)
        {
            file_capture_stop(context);
        }

After that, the signature calculation starts and sending an event due to the file type is never considered.

I have found a workaround and have included a piece of code when signature calculation finishes in order to send an 
event in case the File type has been identified. I'm not sure if this is the behavior that the snort guys pretended to 
develop at this point, but I've considered it a good place to include the file type event.

This can be useful for someone who, for instance, intends to capture files and be alerted when a file type shows up.

You can find the patch following the link below:

https://github.com/redBorder/snort/commit/2ef66122f7d5a317a9d7f5a4602f5bbbd01ab37c

Best Regards,

Pablo Cantos
redborder.org<http://redborder.org> / pcantos () redborder org<mailto:pcantos () redborder org>

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y

_______________________________________________
Snort-devel mailing list
Snort-devel () lists sourceforge net
https://lists.sourceforge.net/lists/listinfo/snort-devel
Archive:
http://sourceforge.net/mailarchive/forum.php?forum_name=snort-devel

Please visit http://blog.snort.org for the latest news about Snort!