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

@@ -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):

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);

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 {

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);

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 }
 };
 

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

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,

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)

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;

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))

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;
 			}

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);
 	}
 }