Rewrite m_denychans.

- The configuration is now validated on ReadConfig and parsed into
  objects.

- Allow redirecting to channels that don't yet exist. There is no
  technical reason to not allow this and I believe that it would
  be a lot less confusing to users if we allowed this.

- Flatten a bunch of nested if statements.

1 files changed, 123 insertions, 69 deletions

diff --git a/src/modules/m_denychans.cpp b/src/modules/m_denychans.cpp
index 467c8a03b..cc4172529 100644
--- a/src/modules/m_denychans.cpp
+++ b/src/modules/m_denychans.cpp
@@ -1,6 +1,7 @@
 /*
  * InspIRCd -- Internet Relay Chat Daemon
  *
+ *.  Copyright (C) 2018 Peter Powell <petpow@saberuk.com>
  *   Copyright (C) 2009 Daniel De Graaf <danieldg@inspircd.org>
  *   Copyright (C) 2007-2008 Robin Burchell <robin+git@viroteck.net>
  *   Copyright (C) 2007 Dennis Friis <peavey@inspircd.org>
@@ -22,58 +23,116 @@
 
 #include "inspircd.h"
 
+enum
+{
+	// InspIRCd-specific.
+	ERR_BADCHANNEL = 926
+};
+
+struct BadChannel
+{
+	bool allowopers;
+	std::string name;
+	std::string reason;
+	std::string redirect;
+
+	BadChannel(const std::string& Name, const std::string& Redirect, const std::string& Reason, bool AllowOpers)
+		: allowopers(AllowOpers)
+		, name(Name)
+		, reason(Reason)
+		, redirect(Redirect)
+	{
+	}
+};
+
+typedef std::vector<BadChannel> BadChannels;
+typedef std::vector<std::string> GoodChannels;
+
 class ModuleDenyChannels : public Module
 {
+ private:
+	BadChannels badchannels;
+	GoodChannels goodchannels;
+	UserModeReference antiredirectmode;
 	ChanModeReference redirectmode;
 
  public:
 	ModuleDenyChannels()
-		: redirectmode(this, "redirect")
+		: antiredirectmode(this, "antiredirect")
+		, redirectmode(this, "redirect")
 	{
 	}
 
 	void ReadConfig(ConfigStatus& status) CXX11_OVERRIDE
 	{
-		/* check for redirect validity and loops/chains */
-		ConfigTagList tags = ServerInstance->Config->ConfTags("badchan");
+		GoodChannels goodchans;
+		ConfigTagList tags = ServerInstance->Config->ConfTags("goodchan");
+		for (ConfigIter iter = tags.first; iter != tags.second; ++iter)
+		{
+			ConfigTag* tag = iter->second;
+
+			// Ensure that we have the <goodchan:name> parameter.
+			const std::string name = tag->getString("name");
+			if (name.empty())
+				throw ModuleException("<goodchan:name> is a mandatory field, at " + tag->getTagLocation());
+
+			goodchans.push_back(name);
+		}
+
+		BadChannels badchans;
+		tags = ServerInstance->Config->ConfTags("badchan");
 		for (ConfigIter i = tags.first; i != tags.second; ++i)
 		{
-			std::string name = i->second->getString("name");
-			std::string redirect = i->second->getString("redirect");
+			ConfigTag* tag = i->second;
+
+			// Ensure that we have the <badchan:name> parameter.
+			const std::string name = tag->getString("name");
+			if (name.empty())
+				throw ModuleException("<badchan:name> is a mandatory field, at " + tag->getTagLocation());
 
+			// Ensure that we have the <badchan:reason> parameter.
+			const std::string reason = tag->getString("reason");
+			if (reason.empty())
+				throw ModuleException("<badchan:reason> is a mandatory field, at " + tag->getTagLocation());
+
+			const std::string redirect = tag->getString("redirect");
 			if (!redirect.empty())
 			{
-
+				// Ensure that <badchan:redirect> contains a channel name.
 				if (!ServerInstance->IsChannel(redirect))
-				{
-					if (status.srcuser)
-						status.srcuser->WriteNotice("Invalid badchan redirect '" + redirect + "'");
-					throw ModuleException("Invalid badchan redirect, not a channel");
-				}
-
-				for (ConfigIter j = tags.first; j != tags.second; ++j)
-				{
-					if (InspIRCd::Match(redirect, j->second->getString("name")))
-					{
-						bool goodchan = false;
-						ConfigTagList goodchans = ServerInstance->Config->ConfTags("goodchan");
-						for (ConfigIter k = goodchans.first; k != goodchans.second; ++k)
-						{
-							if (InspIRCd::Match(redirect, k->second->getString("name")))
-								goodchan = true;
-						}
-
-						if (!goodchan)
-						{
-							/* <badchan:redirect> is a badchan */
-							if (status.srcuser)
-								status.srcuser->WriteNotice("Badchan " + name + " redirects to badchan " + redirect);
-							throw ModuleException("Badchan redirect loop");
-						}
-					}
-				}
+					throw ModuleException("<badchan:redirect> is not a valid channel name, at " + tag->getTagLocation());
+
+				// We defer the rest of the validation of the redirect channel until we have
+				// finished parsing all of the badchans.
 			}
+
+			badchans.push_back(BadChannel(name, redirect, reason, tag->getBool("allowopers")));
+		}
+
+		// Now we have all of the badchan information recorded we can check that all redirect
+		// channels can actually be redirected to.
+		for (BadChannels::const_iterator i = badchans.begin(); i != badchans.end(); ++i)
+		{
+			const BadChannel& badchan = *i;
+
+			// If there is no redirect channel we have nothing to do.
+			if (badchan.redirect.empty())
+				continue;
+
+			// If the redirect channel is whitelisted then it is okay.
+			for (GoodChannels::const_iterator j = goodchans.begin(); j != goodchans.end(); ++j)
+				if (InspIRCd::Match(badchan.redirect, *j))
+					continue;
+
+			// If the redirect channel is not blacklisted then it is okay.
+			for (BadChannels::const_iterator j = badchans.begin(); j != badchans.end(); ++j)
+				if (InspIRCd::Match(badchan.redirect, j->name))
+					throw ModuleException("<badchan:redirect> cannot be a blacklisted channel name");
 		}
+
+		// The config file contained no errors so we can apply the new configuration.
+		badchannels.swap(badchans);
+		goodchannels.swap(goodchans);
 	}
 
 	Version GetVersion() CXX11_OVERRIDE
@@ -84,45 +143,40 @@ class ModuleDenyChannels : public Module
 
 	ModResult OnUserPreJoin(LocalUser* user, Channel* chan, const std::string& cname, std::string& privs, const std::string& keygiven) CXX11_OVERRIDE
 	{
-		ConfigTagList tags = ServerInstance->Config->ConfTags("badchan");
-		for (ConfigIter j = tags.first; j != tags.second; ++j)
+		for (BadChannels::const_iterator j = badchannels.begin(); j != badchannels.end(); ++j)
 		{
-			if (InspIRCd::Match(cname, j->second->getString("name")))
-			{
-				if (user->IsOper() && j->second->getBool("allowopers"))
-				{
+			const BadChannel& badchan = *j;
+
+			// If the channel does not match the current entry we have nothing else to do.
+			if (!InspIRCd::Match(cname, badchan.name))
+				continue;
+
+			// If the user is an oper and opers are allowed to enter this blacklisted channel
+			// then allow the join.
+			if (user->IsOper() && badchan.allowopers)
+				return MOD_RES_PASSTHRU;
+
+			// If the channel matches a whitelist then allow the join.
+			for (GoodChannels::const_iterator i = goodchannels.begin(); i != goodchannels.end(); ++i)
+				if (InspIRCd::Match(cname, *i))
 					return MOD_RES_PASSTHRU;
-				}
-				else
-				{
-					std::string reason = j->second->getString("reason");
-					std::string redirect = j->second->getString("redirect");
-
-					ConfigTagList goodchans = ServerInstance->Config->ConfTags("goodchan");
-					for (ConfigIter i = goodchans.first; i != goodchans.second; ++i)
-					{
-						if (InspIRCd::Match(cname, i->second->getString("name")))
-						{
-							return MOD_RES_PASSTHRU;
-						}
-					}
-
-					if (ServerInstance->IsChannel(redirect))
-					{
-						/* simple way to avoid potential loops: don't redirect to +L channels */
-						Channel *newchan = ServerInstance->FindChan(redirect);
-						if ((!newchan) || (!newchan->IsModeSet(redirectmode)))
-						{
-							user->WriteNumeric(926, cname, InspIRCd::Format("Channel %s is forbidden, redirecting to %s: %s", cname.c_str(), redirect.c_str(), reason.c_str()));
-							Channel::JoinUser(user, redirect);
-							return MOD_RES_DENY;
-						}
-					}
-
-					user->WriteNumeric(926, cname, InspIRCd::Format("Channel %s is forbidden: %s", cname.c_str(), reason.c_str()));
-					return MOD_RES_DENY;
-				}
+
+			// If there is no redirect chan, the user has enabled the antiredirect mode, or
+			// the target channel redirects elsewhere we just tell the user and deny the join.
+			Channel* target = NULL;
+			if (badchan.redirect.empty() || user->IsModeSet(antiredirectmode)
+				|| ((target = ServerInstance->FindChan(badchan.redirect)) && target->IsModeSet(redirectmode)))
+			{
+				user->WriteNumeric(ERR_BADCHANNEL, cname, InspIRCd::Format("Channel %s is forbidden: %s",
+					cname.c_str(), badchan.reason.c_str()));
+				return MOD_RES_DENY;
 			}
+
+			// Redirect the user to the target channel.
+			user->WriteNumeric(ERR_BADCHANNEL, cname, InspIRCd::Format("Channel %s is forbidden, redirecting to %s: %s",
+				cname.c_str(), badchan.redirect.c_str(), badchan.reason.c_str()));
+			Channel::JoinUser(user, badchan.redirect);
+			return MOD_RES_DENY;
 		}
 		return MOD_RES_PASSTHRU;
 	}