File preprocessor: suspected bug found and solved

[Pablo Cantos Polaino <pcantos () redborder org>]

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 / 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!