From 80e80132826569444e690a177f5bfc5f603fada1 Mon Sep 17 00:00:00 2001 From: Sadie Powell Date: Sun, 17 Apr 2022 11:02:42 +0100 Subject: [PATCH] Remove ValidateParam and rename CanonicalizeParam. There's basically no safe way to handle a malformed list mode sent by a remote server without causing a desync. Its probably for the best if we just only apply validation to locally added list modes entries. --- include/listmode.h | 11 +------- src/coremods/core_channel/core_channel.h | 2 +- src/coremods/core_channel/modes.cpp | 2 +- src/listmode.cpp | 32 +++--------------------- src/modules/m_banexception.cpp | 2 +- src/modules/m_chanfilter.cpp | 3 ++- src/modules/m_exemptchanops.cpp | 3 ++- src/modules/m_inviteexception.cpp | 2 +- 8 files changed, 13 insertions(+), 44 deletions(-) diff --git a/include/listmode.h b/include/listmode.h index 7e3b49a86..953957646 100644 --- a/include/listmode.h +++ b/include/listmode.h @@ -156,16 +156,7 @@ public: * @param parameter The parameter that the user specified. * @return True if the parameter is valid; otherwise, false. */ - virtual bool CanonicalizeParam(LocalUser* user, Channel* channel, std::string& parameter); - - /** Validate parameters. - * Overridden by implementing module. - * @param user Source user adding the parameter - * @param channel Channel the parameter is being added to - * @param parameter The actual parameter being added - * @return true if the parameter is valid - */ - virtual bool ValidateParam(User* user, Channel* channel, const std::string& parameter); + virtual bool ValidateParam(LocalUser* user, Channel* channel, std::string& parameter); /** @copydoc ModeHandler::OnParameterMissing */ void OnParameterMissing(User* user, User* dest, Channel* channel) override; diff --git a/src/coremods/core_channel/core_channel.h b/src/coremods/core_channel/core_channel.h index 47e6a4fa2..2e86a2988 100644 --- a/src/coremods/core_channel/core_channel.h +++ b/src/coremods/core_channel/core_channel.h @@ -114,7 +114,7 @@ private: ExtBan::ManagerRef extbanmgr; public: ModeChannelBan(Module* Creator); - bool CanonicalizeParam(LocalUser* user, Channel* channel, std::string& parameter) override; + bool ValidateParam(LocalUser* user, Channel* channel, std::string& parameter) override; }; class ModeChannelKey final diff --git a/src/coremods/core_channel/modes.cpp b/src/coremods/core_channel/modes.cpp index 69cdb55dd..d8afc1261 100644 --- a/src/coremods/core_channel/modes.cpp +++ b/src/coremods/core_channel/modes.cpp @@ -27,7 +27,7 @@ ModeChannelBan::ModeChannelBan(Module* Creator) syntax = ""; } -bool ModeChannelBan::CanonicalizeParam(LocalUser* user, Channel* channel, std::string& parameter) +bool ModeChannelBan::ValidateParam(LocalUser* user, Channel* channel, std::string& parameter) { if (!extbanmgr || !extbanmgr->Canonicalize(parameter)) ModeParser::CleanMask(parameter); diff --git a/src/listmode.cpp b/src/listmode.cpp index e224f1168..6ba6d1771 100644 --- a/src/listmode.cpp +++ b/src/listmode.cpp @@ -157,7 +157,7 @@ ModeAction ListModeBase::OnModeChange(User* source, User*, Channel* channel, Mod { // Try to canonicalise the parameter locally. LocalUser* lsource = IS_LOCAL(source); - if (lsource && !CanonicalizeParam(lsource, channel, change.param)) + if (lsource && !ValidateParam(lsource, channel, change.param)) return MODEACTION_DENY; // If there was no list @@ -188,27 +188,8 @@ ModeAction ListModeBase::OnModeChange(User* source, User*, Channel* channel, Mod return MODEACTION_DENY; } - /* Ok, it *could* be allowed, now give someone subclassing us - * a chance to validate the parameter. - * The param is passed by reference, so they can both modify it - * and tell us if we allow it or not. - * - * eg, the subclass could: - * 1) allow - * 2) 'fix' parameter and then allow - * 3) deny - */ - if (ValidateParam(source, channel, change.param)) - { - // And now add the mask onto the list... - cd->list.emplace_back(change.param, change.set_by.value_or(source->nick), change.set_at.value_or(ServerInstance->Time())); - return MODEACTION_ALLOW; - } - else - { - /* If they deny it they have the job of giving an error message */ - return MODEACTION_DENY; - } + cd->list.emplace_back(change.param, change.set_by.value_or(source->nick), change.set_at.value_or(ServerInstance->Time())); + return MODEACTION_ALLOW; } else { @@ -231,12 +212,7 @@ ModeAction ListModeBase::OnModeChange(User* source, User*, Channel* channel, Mod } } -bool ListModeBase::CanonicalizeParam(LocalUser* user, Channel* channel, std::string& parameter) -{ - return true; -} - -bool ListModeBase::ValidateParam(User* user, Channel* channel, const std::string& parameter) +bool ListModeBase::ValidateParam(LocalUser* user, Channel* channel, std::string& parameter) { return true; } diff --git a/src/modules/m_banexception.cpp b/src/modules/m_banexception.cpp index b975ab7c9..9234c1b6f 100644 --- a/src/modules/m_banexception.cpp +++ b/src/modules/m_banexception.cpp @@ -50,7 +50,7 @@ public: syntax = ""; } - bool CanonicalizeParam(LocalUser* user, Channel* channel, std::string& parameter) override + bool ValidateParam(LocalUser* user, Channel* channel, std::string& parameter) override { if (!extbanmgr || !extbanmgr->Canonicalize(parameter)) ModeParser::CleanMask(parameter); diff --git a/src/modules/m_chanfilter.cpp b/src/modules/m_chanfilter.cpp index c399be68a..a45440a0c 100644 --- a/src/modules/m_chanfilter.cpp +++ b/src/modules/m_chanfilter.cpp @@ -47,8 +47,9 @@ public: syntax = ""; } - bool ValidateParam(User* user, Channel* chan, const std::string& parameter) override + bool ValidateParam(LocalUser* user, Channel* chan, std::string& parameter) override { + // We only enforce the length restriction against local users to avoid causing a desync. if (parameter.length() > maxlen) { user->WriteNumeric(Numerics::InvalidModeParameter(chan, this, parameter, "Entry is too long for the spamfilter list.")); diff --git a/src/modules/m_exemptchanops.cpp b/src/modules/m_exemptchanops.cpp index 89c1688fc..c384e723f 100644 --- a/src/modules/m_exemptchanops.cpp +++ b/src/modules/m_exemptchanops.cpp @@ -81,8 +81,9 @@ public: return MOD_RES_DENY; } - bool ValidateParam(User* user, Channel* chan, const std::string& parameter) override + bool ValidateParam(LocalUser* user, Channel* chan, std::string& parameter) override { + // We only enforce the format restriction against local users to avoid causing a desync. std::string restriction; std::string prefix; if (!ParseEntry(parameter, restriction, prefix)) diff --git a/src/modules/m_inviteexception.cpp b/src/modules/m_inviteexception.cpp index 8db100230..b2f5c8afa 100644 --- a/src/modules/m_inviteexception.cpp +++ b/src/modules/m_inviteexception.cpp @@ -48,7 +48,7 @@ public: syntax = ""; } - bool CanonicalizeParam(LocalUser* user, Channel* channel, std::string& parameter) override + bool ValidateParam(LocalUser* user, Channel* channel, std::string& parameter) override { if (!extbanmgr || !extbanmgr->Canonicalize(parameter)) ModeParser::CleanMask(parameter);