From ec196b39681128a88fdd61b5ee3d61c9a4836d7e Mon Sep 17 00:00:00 2001 From: BlackLight Date: Tue, 16 Nov 2010 19:18:08 +0100 Subject: [PATCH] Fixing a multiple access to output database bug --- alert_parser.c | 3 +-- cluster.c | 4 +++- correlation.c | 1 - db.c | 1 - db.h | 10 ++++++++++ mysql.c | 25 +++++++++++++++++++++++++ neural.c | 23 ++++++++++++++++++----- outdb.c | 44 ++++++++++++++++++++++---------------------- spp_ai.c | 1 - spp_ai.h | 5 +++++ stream.c | 10 ++++++---- webserv.c | 1 - 12 files changed, 90 insertions(+), 38 deletions(-) diff --git a/alert_parser.c b/alert_parser.c index f5cc6db..f46e3d1 100644 --- a/alert_parser.c +++ b/alert_parser.c @@ -19,7 +19,6 @@ #include "spp_ai.h" -#include #include #include #include @@ -325,7 +324,7 @@ AI_file_alertparser_thread ( void* arg ) free ( matches ); matches = NULL; } else { - AI_fatal_err ( "Parse error: a line in the alert log cannot be associated to an alert block", __FILE__, __LINE__ ); + _dpd.errMsg ( "Error: a line in the alert log cannot be associated to an alert block", __FILE__, __LINE__ ); } } else if ( preg_match ( "\\[Priority:\\s*([0-9]+)\\]", line, &matches, &nmatches) > 0 ) { alert->priority = (unsigned short) strtoul ( matches[0], NULL, 10 ); diff --git a/cluster.c b/cluster.c index 894cfcc..cfa5eed 100644 --- a/cluster.c +++ b/cluster.c @@ -21,7 +21,6 @@ #include #include -#include #include #include @@ -481,6 +480,9 @@ __AI_cluster_thread ( void* arg ) alert_log = NULL; } + /* get_alerts() is a function pointer that can point to the function for getting the alerts from + * the plain alert log file or from the database. Calling it the source of the alerts is + * completely transparent to this level */ if ( !( alert_log = get_alerts() )) { pthread_mutex_unlock ( &mutex ); diff --git a/correlation.c b/correlation.c index 0bfb2bf..a607bec 100644 --- a/correlation.c +++ b/correlation.c @@ -22,7 +22,6 @@ #include #include #include -#include #include #include #include diff --git a/db.c b/db.c index ac2c473..0006749 100644 --- a/db.c +++ b/db.c @@ -22,7 +22,6 @@ #include "db.h" -#include #include #include diff --git a/db.h b/db.h index 38ac2c8..83fdf19 100644 --- a/db.h +++ b/db.h @@ -34,16 +34,26 @@ #define DB_fetch_row mysql_fetch_row #define DB_free_result mysql_free_result #define DB_escape_string mysql_do_escape_string + #define DB_do_error mysql_do_error + #define DB_is_gone mysql_is_gone #define DB_close mysql_do_close #define DB_out_init mysql_do_out_init #define DB_is_out_init mysql_is_out_init #define DB_out_query mysql_do_out_query #define DB_out_escape_string mysql_do_out_escape_string + #define DB_do_out_error mysql_do_out_error + #define DB_is_out_gone mysql_is_out_gone #define DB_out_close mysql_do_out_close DB_result* DB_query ( const char* ); DB_result* DB_out_query ( const char* ); + + const char* DB_do_error(); + const char* DB_do_out_error(); + + BOOL DB_is_gone(); + BOOL DB_is_out_gone(); #endif #ifdef HAVE_LIBPQ diff --git a/mysql.c b/mysql.c index 8416091..9a6631d 100644 --- a/mysql.c +++ b/mysql.c @@ -21,6 +21,7 @@ #ifdef HAVE_LIBMYSQLCLIENT #include +#include /** \defgroup mysql Module for the interface with a MySQL DBMS * @{ */ @@ -124,6 +125,18 @@ mysql_do_escape_string ( char **to, const char *from, unsigned long length ) return mysql_real_escape_string ( db, *to, from, length ); } +const char* +mysql_do_error () +{ + return mysql_error ( db ); +} + +BOOL +mysql_is_gone () +{ + return (( mysql_errno ( db ) == CR_SERVER_GONE_ERROR ) || ( mysql_errno ( db ) == CR_SERVER_LOST )); +} + void mysql_do_close () { @@ -156,6 +169,18 @@ mysql_do_out_escape_string ( char **to, const char *from, unsigned long length ) return mysql_real_escape_string ( outdb, *to, from, length ); } +const char* +mysql_do_out_error () +{ + return mysql_error ( outdb ); +} + +BOOL +mysql_is_out_gone () +{ + return (( mysql_errno ( outdb ) == CR_SERVER_GONE_ERROR ) || ( mysql_errno ( outdb ) == CR_SERVER_LOST )); +} + void mysql_do_out_close () { diff --git a/neural.c b/neural.c index 8427309..14a157c 100644 --- a/neural.c +++ b/neural.c @@ -30,7 +30,6 @@ #include #include #include -#include #include #include #include @@ -68,20 +67,24 @@ AI_neural_correlation_weight () k = (double) config->alert_correlation_weight / HYPERBOLIC_TANGENT_SOLUTION; snprintf ( query, sizeof ( query ), "SELECT count(*) FROM %s", outdb_config[ALERTS_TABLE] ); + pthread_mutex_lock ( &outdb_mutex ); if ( !DB_out_init() ) { + pthread_mutex_unlock ( &outdb_mutex ); AI_fatal_err ( "Unable to connect to the database specified in module configuration", __FILE__, __LINE__ ); } if ( !( res = (DB_result) DB_out_query ( query ))) { - AI_fatal_err ( "AIPreproc: Query error", __FILE__, __LINE__ ); + pthread_mutex_unlock ( &outdb_mutex ); + return 0; } row = (DB_row) DB_fetch_row ( res ); x = strtod ( row[0], NULL ); DB_free_result ( res ); + pthread_mutex_unlock ( &outdb_mutex ); return (( exp(x/k) - exp(-x/k) ) / ( exp(x/k) + exp(-x/k) )); } /* ----- end of function AI_neural_correlation_weight ----- */ @@ -183,7 +186,10 @@ AI_alert_neural_som_correlation ( const AI_snort_alert *a, const AI_snort_alert /* The timestamp of this alert is computed like the average timestamp of the grouped alerts */ for ( i=1; i < a->grouped_alerts_count; i++ ) { - time_sum += (unsigned long long int) a->grouped_alerts[i-1]->timestamp; + if ( a->grouped_alerts[i-1] ) + { + time_sum += (unsigned long long int) a->grouped_alerts[i-1]->timestamp; + } } t1.timestamp = (time_t) ( time_sum / a->grouped_alerts_count ); @@ -199,7 +205,10 @@ AI_alert_neural_som_correlation ( const AI_snort_alert *a, const AI_snort_alert for ( i=1; i < b->grouped_alerts_count; i++ ) { - time_sum += (unsigned long long int) b->grouped_alerts[i-1]->timestamp; + if ( b->grouped_alerts[i-1] ) + { + time_sum += (unsigned long long int) b->grouped_alerts[i-1]->timestamp; + } } t2.timestamp = (time_t) ( time_sum / b->grouped_alerts_count ); @@ -224,8 +233,11 @@ __AI_som_train () DB_row row; AI_som_alert_tuple *tuples = NULL; + pthread_mutex_lock ( &outdb_mutex ); + if ( !DB_out_init() ) { + pthread_mutex_unlock ( &outdb_mutex ); AI_fatal_err ( "Unable to connect to the database specified in module configuration", __FILE__, __LINE__ ); } @@ -251,9 +263,11 @@ __AI_som_train () if ( !( res = (DB_result) DB_out_query ( query ))) { + pthread_mutex_unlock ( &outdb_mutex ); return; } + pthread_mutex_unlock ( &outdb_mutex ); num_rows = DB_num_rows ( res ); if ( num_rows == 0 ) @@ -295,7 +309,6 @@ __AI_som_train () } DB_free_result ( res ); - pthread_mutex_lock ( &neural_mutex ); if ( !net ) diff --git a/outdb.c b/outdb.c index a312018..77ea502 100644 --- a/outdb.c +++ b/outdb.c @@ -30,7 +30,6 @@ #include "uthash.h" #include -#include /** Hash table built as cache for the couple of alerts already belonging to the same cluster, * for avoiding more queries on the database*/ @@ -41,7 +40,7 @@ typedef struct { } AI_couples_cache; /** Mutex object, for managing concurrent thread access to the database */ -PRIVATE pthread_mutex_t mutex; +pthread_mutex_t outdb_mutex; PRIVATE AI_couples_cache *couples_cache = NULL; /** @@ -51,7 +50,7 @@ PRIVATE AI_couples_cache *couples_cache = NULL; void AI_outdb_mutex_initialize () { - pthread_mutex_init ( &mutex, NULL ); + pthread_mutex_init ( &outdb_mutex, NULL ); } /* ----- end of function AI_outdb_mutex_initialize ----- */ /** @@ -80,7 +79,7 @@ AI_store_alert_to_db_thread ( void *arg ) DB_row row; AI_snort_alert *alert = (AI_snort_alert*) arg; - pthread_mutex_lock ( &mutex ); + pthread_mutex_lock ( &outdb_mutex ); if ( !DB_out_init() ) AI_fatal_err ( "Unable to connect to the specified output database", __FILE__, __LINE__ ); @@ -109,13 +108,14 @@ AI_store_alert_to_db_thread ( void *arg ) if ( !( res = (DB_result) DB_out_query ( query ))) { _dpd.logMsg ( "AIPreproc: Warning: error in executing query: '%s'\n", query ); - pthread_mutex_unlock ( &mutex ); + pthread_mutex_unlock ( &outdb_mutex ); pthread_exit ((void*) 0); + return (void*) 0; } if ( !( row = (DB_row) DB_fetch_row ( res ))) { - pthread_mutex_unlock ( &mutex ); + pthread_mutex_unlock ( &outdb_mutex ); pthread_exit ((void*) 0); } @@ -145,13 +145,13 @@ AI_store_alert_to_db_thread ( void *arg ) if ( !( res = (DB_result) DB_out_query ( query ))) { _dpd.logMsg ( "AIPreproc: Warning: error in executing query: '%s'\n", query ); - pthread_mutex_unlock ( &mutex ); + pthread_mutex_unlock ( &outdb_mutex ); pthread_exit ((void*) 0); } if ( !( row = (DB_row) DB_fetch_row ( res ))) { - pthread_mutex_unlock ( &mutex ); + pthread_mutex_unlock ( &outdb_mutex ); pthread_exit ((void*) 0); } @@ -211,13 +211,13 @@ AI_store_alert_to_db_thread ( void *arg ) if ( !( res = (DB_result) DB_out_query ( query ))) { _dpd.logMsg ( "AIPreproc: Warning: error in executing query: '%s'\n", query ); - pthread_mutex_unlock ( &mutex ); + pthread_mutex_unlock ( &outdb_mutex ); pthread_exit ((void*) 0); } if ( !( row = (DB_row) DB_fetch_row ( res ))) { - pthread_mutex_unlock ( &mutex ); + pthread_mutex_unlock ( &outdb_mutex ); pthread_exit ((void*) 0); } @@ -274,7 +274,7 @@ AI_store_alert_to_db_thread ( void *arg ) } } - pthread_mutex_unlock ( &mutex ); + pthread_mutex_unlock ( &outdb_mutex ); pthread_exit ((void*) 0); return (void*) 0; } /* ----- end of function AI_store_alert_to_db_thread ----- */ @@ -303,7 +303,7 @@ AI_store_cluster_to_db_thread ( void *arg ) DB_row row; BOOL new_cluster = false; - pthread_mutex_lock ( &mutex ); + pthread_mutex_lock ( &outdb_mutex ); /* Check if the couple of alerts is already in our cache, so it already * belongs to the same cluster. If so, just return */ @@ -311,7 +311,7 @@ AI_store_cluster_to_db_thread ( void *arg ) if ( found ) { - pthread_mutex_unlock ( &mutex ); + pthread_mutex_unlock ( &outdb_mutex ); pthread_exit ((void*) 0); return (void*) 0; } @@ -323,7 +323,7 @@ AI_store_cluster_to_db_thread ( void *arg ) /* If one of the two alerts has no alert_id, simply return */ if ( !alerts_couple->alert1->alert_id || !alerts_couple->alert2->alert_id ) { - pthread_mutex_unlock ( &mutex ); + pthread_mutex_unlock ( &outdb_mutex ); pthread_exit ((void*) 0); return (void*) 0; } @@ -337,14 +337,14 @@ AI_store_cluster_to_db_thread ( void *arg ) if ( !( res = (DB_result) DB_out_query ( query ))) { _dpd.logMsg ( "AIPreproc: Warning: error in executing query: '%s'\n", query ); - pthread_mutex_unlock ( &mutex ); + pthread_mutex_unlock ( &outdb_mutex ); pthread_exit ((void*) 0); return (void*) 0; } if ( !( row = (DB_row) DB_fetch_row ( res ))) { - pthread_mutex_unlock ( &mutex ); + pthread_mutex_unlock ( &outdb_mutex ); pthread_exit ((void*) 0); return (void*) 0; } @@ -384,7 +384,7 @@ AI_store_cluster_to_db_thread ( void *arg ) found->alerts_couple = alerts_couple; found->cluster_id = cluster1; HASH_ADD ( hh, couples_cache, alerts_couple, sizeof ( AI_alerts_couple ), found ); - pthread_mutex_unlock ( &mutex ); + pthread_mutex_unlock ( &outdb_mutex ); pthread_exit ((void*) 0); return (void*) 0; } @@ -417,14 +417,14 @@ AI_store_cluster_to_db_thread ( void *arg ) if ( !( res = (DB_result) DB_out_query ( query ))) { _dpd.logMsg ( "AIPreproc: Warning: error in executing query: '%s'\n", query ); - pthread_mutex_unlock ( &mutex ); + pthread_mutex_unlock ( &outdb_mutex ); pthread_exit ((void*) 0); return (void*) 0; } if ( !( row = (DB_row) DB_fetch_row ( res ))) { - pthread_mutex_unlock ( &mutex ); + pthread_mutex_unlock ( &outdb_mutex ); pthread_exit ((void*) 0); return (void*) 0; } @@ -468,7 +468,7 @@ AI_store_cluster_to_db_thread ( void *arg ) found->cluster_id = cluster1; HASH_ADD ( hh, couples_cache, alerts_couple, sizeof ( AI_alerts_couple ), found ); - pthread_mutex_unlock ( &mutex ); + pthread_mutex_unlock ( &outdb_mutex ); pthread_exit ((void*) 0); return (void*) 0; } /* ----- end of function AI_store_cluster_to_db_thread ----- */ @@ -485,7 +485,7 @@ AI_store_correlation_to_db_thread ( void *arg ) char query[1024] = { 0 }; AI_alert_correlation *corr = (AI_alert_correlation*) arg; - pthread_mutex_lock ( &mutex ); + pthread_mutex_lock ( &outdb_mutex ); /* Initialize the database (it just does nothing if it is already initialized) */ if ( !DB_out_init() ) @@ -501,7 +501,7 @@ AI_store_correlation_to_db_thread ( void *arg ) corr->correlation ); DB_free_result ((DB_result) DB_out_query ( query )); - pthread_mutex_unlock ( &mutex ); + pthread_mutex_unlock ( &outdb_mutex ); pthread_exit ((void*) 0); return 0; } /* ----- end of function AI_store_correlation_to_db_thread ----- */ diff --git a/spp_ai.c b/spp_ai.c index fb0cbd3..d76d15a 100644 --- a/spp_ai.c +++ b/spp_ai.c @@ -22,7 +22,6 @@ #include "sf_preproc_info.h" #include -#include #include #include diff --git a/spp_ai.h b/spp_ai.h index 56a3500..da12f85 100644 --- a/spp_ai.h +++ b/spp_ai.h @@ -28,6 +28,8 @@ #include "sf_dynamic_preprocessor.h" #include "uthash.h" +#include + #define PRIVATE static /** Default interval in seconds for the thread cleaning up TCP streams */ @@ -524,6 +526,9 @@ extern AI_snort_alert **alerts_pool; /** Number of alerts contained in the buffer to be serialized */ extern unsigned int alerts_pool_count; +/** Mutex variable for writing on the output database */ +extern pthread_mutex_t outdb_mutex; + /** Configuration of the module */ extern AI_config *config; diff --git a/stream.c b/stream.c index e3b0053..a1f44e8 100644 --- a/stream.c +++ b/stream.c @@ -19,7 +19,6 @@ #include "spp_ai.h" -#include #include #include #include @@ -43,18 +42,21 @@ PRIVATE void __AI_stream_free ( struct pkt_info* stream ) { struct pkt_info *tmp = NULL; - char ip [ INET_ADDRSTRLEN ]; + /* If the provided stream is empty, or the hash table contains no element, just return */ if ( !stream || !hash || HASH_COUNT(hash) == 0 ) return; + /* Lock the mutex over the hash table and search for a stream having the provided key */ pthread_mutex_lock ( &hash_mutex ); HASH_FIND ( hh, hash, &(stream->key), sizeof(struct pkt_key), tmp ); pthread_mutex_unlock ( &hash_mutex ); + /* If that key is not there, just return */ if ( !tmp ) return; + /* If the stream has no IP or TCP header, return */ if ( stream->pkt ) { if ( !stream->pkt->ip4_header ) @@ -64,12 +66,12 @@ __AI_stream_free ( struct pkt_info* stream ) return; } - inet_ntop ( AF_INET, &(stream->key.src_ip), ip, INET_ADDRSTRLEN ); - + /* Remove the stream from the hash table */ pthread_mutex_lock ( &hash_mutex ); HASH_DEL ( hash, stream ); pthread_mutex_unlock ( &hash_mutex ); + /* Remove all the packets contained in the stream */ while ( stream ) { tmp = stream->next; diff --git a/webserv.c b/webserv.c index d69f4ff..dd0d731 100644 --- a/webserv.c +++ b/webserv.c @@ -20,7 +20,6 @@ #include "spp_ai.h" #include -#include #include #include #include