From 4c3d2a6d6dde0656959a3d0959d19da9bebc15af Mon Sep 17 00:00:00 2001 From: Bram Matthys Date: Tue, 11 Jul 2023 14:16:12 +0200 Subject: [PATCH] Fix write bug in tkldb and add spamfilter::action stop. The spamfilter::action stop ill prevent processing other spamfilters. This would normally be a bit unusual, and potentially dangerous when you do exclude things this way, but can be useful in some circumstances. Stopping only affects the same type of spamfilters (general or central spamfilters), so they don't interfere. The tkldb write DB bug had to do with that it was processing central spamfilters, which should be skipped just like config based spamfilters were already skipped. --- doc/RELEASE-NOTES.md | 1 + include/h.h | 2 +- include/struct.h | 1 + src/api-efunctions.c | 2 +- src/misc.c | 1 + src/modules/antimixedutf8.c | 2 +- src/modules/antirandom.c | 2 +- src/modules/blacklist.c | 2 +- src/modules/message.c | 2 +- src/modules/tkl.c | 118 ++++++++++++++++++++++++++---------- src/modules/tkldb.c | 2 + src/parse.c | 2 +- 12 files changed, 99 insertions(+), 38 deletions(-) diff --git a/doc/RELEASE-NOTES.md b/doc/RELEASE-NOTES.md index 398cbe237..c3c15d436 100644 --- a/doc/RELEASE-NOTES.md +++ b/doc/RELEASE-NOTES.md @@ -19,6 +19,7 @@ in progress and may not be a stable version. works like a regular [except item](https://www.unrealircd.org/docs/Mask_item). If this matches, then the spamfilter will not run at all (no hit). * The `action` item now supports multiple actions: + * A new action `stop` to stop other spamfilters from processing * A new action `set` to set a TAG on a user, or increasing the value of one * A new action `report` to call a spamreport block, see next. * A new [spamreport { } block](https://www.unrealircd.org/docs/Spamreport_block): diff --git a/include/h.h b/include/h.h index 5b08ac483..f4fa74017 100644 --- a/include/h.h +++ b/include/h.h @@ -843,7 +843,7 @@ extern MODVAR TKL *(*find_tkline_match_zap)(Client *cptr); extern MODVAR void (*tkl_stats)(Client *cptr, int type, const char *para, int *cnt); extern MODVAR void (*tkl_sync)(Client *client); extern MODVAR void (*cmd_tkl)(Client *client, MessageTag *recv_mtags, int parc, const char *parv[]); -extern MODVAR int (*take_action)(Client *client, BanAction *actions, const char *reason, long duration, int take_action_flags); +extern MODVAR int (*take_action)(Client *client, BanAction *actions, const char *reason, long duration, int take_action_flags, int *stopped); extern MODVAR int (*match_spamfilter)(Client *client, const char *str_in, int type, const char *cmd, const char *target, int flags, TKL **rettk); extern MODVAR int (*match_spamfilter_mtags)(Client *client, MessageTag *mtags, const char *cmd); extern MODVAR int (*join_viruschan)(Client *client, TKL *tk, int type); diff --git a/include/struct.h b/include/struct.h index 8b6f6609a..48dc73db9 100644 --- a/include/struct.h +++ b/include/struct.h @@ -1179,6 +1179,7 @@ typedef enum BanActionValue { BAN_ACT_REPORT = 40, // anything above BAN_ACT_SET will will cause a log message to be emitted BAN_ACT_SET = 30, + BAN_ACT_STOP = 5, } BanActionValue; typedef enum VarActionValue { diff --git a/src/api-efunctions.c b/src/api-efunctions.c index 59fbbfb2f..ecd8c95f7 100644 --- a/src/api-efunctions.c +++ b/src/api-efunctions.c @@ -74,7 +74,7 @@ TKL *(*find_tkline_match_zap)(Client *client); void (*tkl_stats)(Client *client, int type, const char *para, int *cnt); void (*tkl_sync)(Client *client); void (*cmd_tkl)(Client *client, MessageTag *mtags, int parc, const char *parv[]); -int (*take_action)(Client *client, BanAction *action, const char *reason, long duration, int take_action_flags); +int (*take_action)(Client *client, BanAction *action, const char *reason, long duration, int take_action_flags, int *stopped); int (*match_spamfilter)(Client *client, const char *str_in, int type, const char *cmd, const char *target, int flags, TKL **rettk); int (*match_spamfilter_mtags)(Client *client, MessageTag *mtags, const char *cmd); int (*join_viruschan)(Client *client, TKL *tk, int type); diff --git a/src/misc.c b/src/misc.c index 9e312b36c..66a39f943 100644 --- a/src/misc.c +++ b/src/misc.c @@ -81,6 +81,7 @@ static BanActTable banacttable[] = { { BAN_ACT_SOFT_WARN, 'W', "soft-warn" }, { BAN_ACT_SET, '1', "set" }, { BAN_ACT_REPORT, 'r', "report" }, + { BAN_ACT_STOP, '0', "stop" }, { 0, 0, 0 } }; diff --git a/src/modules/antimixedutf8.c b/src/modules/antimixedutf8.c index 929facc57..da707c4bd 100644 --- a/src/modules/antimixedutf8.c +++ b/src/modules/antimixedutf8.c @@ -697,7 +697,7 @@ CMD_OVERRIDE_FUNC(override_msg) log_data_string("scripts", logbuf)); /* Take the action */ - retval = take_action(client, cfg.ban_action, cfg.ban_reason, cfg.ban_time, 0); + retval = take_action(client, cfg.ban_action, cfg.ban_reason, cfg.ban_time, 0, NULL); if ((retval == BAN_ACT_WARN) || (retval == BAN_ACT_SOFT_WARN)) { /* no action */ diff --git a/src/modules/antirandom.c b/src/modules/antirandom.c index dce7b573e..add57c062 100644 --- a/src/modules/antirandom.c +++ b/src/modules/antirandom.c @@ -866,7 +866,7 @@ int antirandom_preconnect(Client *client) score = get_spam_score(client); if (score > cfg.threshold) { - int n = take_action(client, cfg.ban_action, cfg.ban_reason, cfg.ban_time, 0); + int n = take_action(client, cfg.ban_action, cfg.ban_reason, cfg.ban_time, 0, NULL); if ((n == BAN_ACT_WARN) || (n == BAN_ACT_SOFT_WARN)) { unreal_log(ULOG_INFO, "antirandom", "ANTIRANDOM_DENIED_USER", client, diff --git a/src/modules/blacklist.c b/src/modules/blacklist.c index bdf0776c3..9a3ed6d0f 100644 --- a/src/modules/blacklist.c +++ b/src/modules/blacklist.c @@ -753,7 +753,7 @@ int blacklist_action(Client *client, char *opernotice, BanAction *ban_action, ch log_data_string("ban_action", ban_actions_to_string(ban_action)), log_data_string("ban_reason", ban_reason), log_data_integer("ban_time", ban_time)); - return take_action(client, ban_action, ban_reason, ban_time, 0); + return take_action(client, ban_action, ban_reason, ban_time, 0, NULL); } void blacklist_hit(Client *client, Blacklist *bl, int reply) diff --git a/src/modules/message.c b/src/modules/message.c index b7e2237ce..7e1dc163c 100644 --- a/src/modules/message.c +++ b/src/modules/message.c @@ -619,7 +619,7 @@ int ban_version(Client *client, const char *text) if (find_tkl_exception(TKL_BAN_VERSION, client)) return 0; /* we are exempt */ - return take_action(client, ban->action, ban->reason, BAN_VERSION_TKL_TIME, 0); + return take_action(client, ban->action, ban->reason, BAN_VERSION_TKL_TIME, 0, NULL); } return 0; diff --git a/src/modules/tkl.c b/src/modules/tkl.c index a6dd1de0c..2fe4a3951 100644 --- a/src/modules/tkl.c +++ b/src/modules/tkl.c @@ -90,7 +90,7 @@ TKL *_find_tkline_match_zap(Client *client); void _tkl_stats(Client *client, int type, const char *para, int *cnt); void _tkl_sync(Client *client); CMD_FUNC(_cmd_tkl); -int _take_action(Client *client, BanAction *action, char *reason, long duration, int take_action_flags); +int _take_action(Client *client, BanAction *action, char *reason, long duration, int take_action_flags, int *stopped); int _match_spamfilter(Client *client, const char *str_in, int type, const char *cmd, const char *target, int flags, TKL **rettk); int _match_spamfilter_mtags(Client *client, MessageTag *mtags, char *cmd); int check_mtag_spamfilters_present(void); @@ -4804,11 +4804,19 @@ void ban_act_set(Client *client, BanAction *action) log_data_integer("value", tag->value)); } -void ban_action_run_all_sets(Client *client, BanAction *action) +void ban_action_run_all_sets_and_stops(Client *client, BanAction *action, int *stopped) { + *stopped = 0; for (; action; action = action->next) + { if (action->action == BAN_ACT_SET) ban_act_set(client, action); + if (action->action == BAN_ACT_STOP) + { + *stopped = 1; + break; + } + } } /** Take an action on the user, such as banning or killing. @@ -4825,12 +4833,15 @@ void ban_action_run_all_sets(Client *client, BanAction *action) * @note Be sure to check IsDead(client) if return value is 1 and you are * considering to continue processing. */ -int _take_action(Client *client, BanAction *actions, char *reason, long duration, int take_action_flags) +int _take_action(Client *client, BanAction *actions, char *reason, long duration, int take_action_flags, int *stopped) { BanAction *action; int previous_highest = 0; int highest = 0; + if (stopped) + *stopped = 0; + for (action = actions; action; action = action->next) { /* If this is a soft action and the user is logged in, then the ban does not apply. */ @@ -4938,6 +4949,10 @@ int _take_action(Client *client, BanAction *actions, char *reason, long duration if (!(take_action_flags & TAKE_ACTION_SKIP_SET)) ban_act_set(client, action); break; + case BAN_ACT_STOP: + if (stopped) + *stopped = 1; + break; default: /* (BAN_ACT_BLOCK, BAN_ACT_SOFT_BLOCK, BAN_ACT_WARN, etc...) */ /* We don't actively do something, up to caller */ @@ -5090,17 +5105,20 @@ static int spamfilter_hide_content(int target) /** Called when a spamfilter is hit. Helper for match_spamfilter(). * @retval 1 to break processing other spamfilters, 0 to continue. */ -static int match_spamfilter_hit(Client *client, const char *str_in, const char *str, int target, - const char *cmd, const char *destination, TKL *tkl, TKL **winner_tkl, - char user_is_exempt_general, char user_is_exempt_central, - int *content_revealed, - char no_stop) +static void match_spamfilter_hit(Client *client, const char *str_in, const char *str, int target, + const char *cmd, const char *destination, TKL *tkl, TKL **winner_tkl, + char user_is_exempt_general, char user_is_exempt_central, + int *stop_processing_general_spamfilters, int *stop_processing_central_spamfilters, + int *content_revealed, + char no_stop_first_match) { int hide_content = spamfilter_hide_content(target); + int stopped; + int highest_action; /* Perhaps it's on the exceptions list? */ if (!*winner_tkl && destination && target_is_spamexcept(destination)) - return 0; /* No problem! */ + return; /* No problem! */ if (match_spamfilter_exempt(tkl, user_is_exempt_general, user_is_exempt_central)) { @@ -5108,7 +5126,8 @@ static int match_spamfilter_hit(Client *client, const char *str_in, const char * } else { tkl->ptr.spamfilter->hits++; - if (highest_ban_action(tkl->ptr.spamfilter->action) > BAN_ACT_SET) + highest_action = highest_ban_action(tkl->ptr.spamfilter->action); + if (highest_action > BAN_ACT_SET) { if (hide_content) { @@ -5134,23 +5153,29 @@ static int match_spamfilter_hit(Client *client, const char *str_in, const char * } /* Run any SET actions */ - ban_action_run_all_sets(client, tkl->ptr.spamfilter->action); + ban_action_run_all_sets_and_stops(client, tkl->ptr.spamfilter->action, &stopped); - /* If we should stop after the first match, we end here... */ - if ((no_stop == 0) && SPAMFILTER_STOP_ON_FIRST_MATCH) - { - *winner_tkl = tkl; - return 1; - } - - /* Otherwise.. we set 'winner_tkl' to the spamfilter with the strongest action. */ + /* Set 'winner_tkl' to the spamfilter with the strongest action. */ if (!*winner_tkl) *winner_tkl = tkl; else *winner_tkl = choose_winning_spamfilter(tkl, *winner_tkl); - /* and continue.. */ - return 0; + /* If set::spamfilter::stop-on-first-match is enabled, then we also stop + * (this is different than an ::action stop but the effect is the same... + */ + if ((no_stop_first_match == 0) && SPAMFILTER_STOP_ON_FIRST_MATCH) + stopped = 1; + + /* Tell caller what actually should be stopped... */ + if (stopped) + { + if (IsCentralSpamfilter(tkl)) + *stop_processing_central_spamfilters = 1; + else + *stop_processing_general_spamfilters = 1; + } + } /** match_spamfilter: executes the spamfilter on the input string. @@ -5177,6 +5202,8 @@ int _match_spamfilter(Client *client, const char *str_in, int target, const char int tags_serial = client->local ? client->local->tags_serial : 0; char user_is_exempt_general = 0; char user_is_exempt_central = 0; + int stop_processing_general_spamfilters = 0; + int stop_processing_central_spamfilters = 0; int content_revealed = 0; if (rettkl) @@ -5210,6 +5237,20 @@ int _match_spamfilter(Client *client, const char *str_in, int target, const char if (!(tkl->ptr.spamfilter->target & target)) continue; + /* Skip spamfilters due to a 'stop' action from an earlier spamfilter + * or set::spamfilter::stop-on-first-match. + * We treat such stops as separate for central & general spamfilters + * so they don't affect each other. + */ + if (IsCentralSpamfilter(tkl)) + { + if (stop_processing_central_spamfilters) + continue; + } else { + if (stop_processing_general_spamfilters) + continue; + } + if ((flags & SPAMFLAG_NOWARN) && only_actions_of_type(tkl->ptr.spamfilter->action, BAN_ACT_WARN)) continue; @@ -5277,15 +5318,12 @@ int _match_spamfilter(Client *client, const char *str_in, int target, const char if (ret) { - if (match_spamfilter_hit(client, str_in, str, target, cmd, destination, - tkl, &winner_tkl, - user_is_exempt_general, user_is_exempt_central, - &content_revealed, - 0)) - { - break; - } - /* otherwise continue.. */ + match_spamfilter_hit(client, str_in, str, target, cmd, destination, + tkl, &winner_tkl, + user_is_exempt_general, user_is_exempt_central, + &stop_processing_general_spamfilters, &stop_processing_general_spamfilters, + &content_revealed, + 0); } } @@ -5296,9 +5334,11 @@ int _match_spamfilter(Client *client, const char *str_in, int target, const char * these are special in the sense that they only run * whenever a tag is changed. */ + stop_processing_general_spamfilters = stop_processing_central_spamfilters = 0; /* reset this */ for (tkl = tklines[tkl_hash('F')]; tkl; tkl = tkl->next) { crule_context context; + if (tkl->ptr.spamfilter->target || (tkl->ptr.spamfilter->match->type != MATCH_NONE) || !tkl->ptr.spamfilter->rule) @@ -5306,6 +5346,21 @@ int _match_spamfilter(Client *client, const char *str_in, int target, const char continue; } + /* Skip spamfilters due to a 'stop' action from an earlier spamfilter + * or set::spamfilter::stop-on-first-match. + * We treat such stops as separate for central & general spamfilters + * so they don't affect each other. + */ + if (IsCentralSpamfilter(tkl)) + { + if (stop_processing_central_spamfilters) + continue; + } else { + if (stop_processing_general_spamfilters) + continue; + } + + if ((flags & SPAMFLAG_NOWARN) && only_actions_of_type(tkl->ptr.spamfilter->action, BAN_ACT_WARN)) continue; @@ -5323,6 +5378,7 @@ int _match_spamfilter(Client *client, const char *str_in, int target, const char match_spamfilter_hit(client, str_in, str, target, cmd, destination, tkl, &winner_tkl, user_is_exempt_general, user_is_exempt_central, + &stop_processing_general_spamfilters, &stop_processing_general_spamfilters, &content_revealed, 1); /* and continue (yes, always, no stopping on first match) */ @@ -5338,7 +5394,7 @@ int _match_spamfilter(Client *client, const char *str_in, int target, const char /* Spamfilter matched */ reason = unreal_decodespace(tkl->ptr.spamfilter->tkl_reason); - ret = take_action(client, tkl->ptr.spamfilter->action, reason, tkl->ptr.spamfilter->tkl_duration, TAKE_ACTION_SKIP_SET); + ret = take_action(client, tkl->ptr.spamfilter->action, reason, tkl->ptr.spamfilter->tkl_duration, TAKE_ACTION_SKIP_SET, NULL); if (!IsDead(client)) { if ((ret == BAN_ACT_BLOCK) || (ret == BAN_ACT_SOFT_BLOCK)) diff --git a/src/modules/tkldb.c b/src/modules/tkldb.c index 46ed35567..5f6141076 100644 --- a/src/modules/tkldb.c +++ b/src/modules/tkldb.c @@ -340,6 +340,8 @@ int write_tkldb(void) { if (tkl->flags & TKL_FLAG_CONFIG) continue; /* config entry */ + if (IsCentralSpamfilter(tkl)) + continue; if (!write_tkline(db, tmpfname, tkl)) // write_tkline() closes the db on errors itself return 0; } diff --git a/src/parse.c b/src/parse.c index d0f991f62..02e7fe3cb 100644 --- a/src/parse.c +++ b/src/parse.c @@ -603,7 +603,7 @@ static void ban_handshake_data_flooder(Client *client) else { /* take_action also takes care of removing any other clients with same host/ip */ - take_action(client, iConf.handshake_data_flood_ban_action, "Handshake data flood detected", iConf.handshake_data_flood_ban_time, 0); + take_action(client, iConf.handshake_data_flood_ban_action, "Handshake data flood detected", iConf.handshake_data_flood_ban_time, 0, NULL); } }