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.
This commit is contained in:
Sadie Powell 2022-04-17 11:02:42 +01:00
parent 3b71730688
commit 80e8013282
8 changed files with 13 additions and 44 deletions

View File

@ -156,16 +156,7 @@ public:
* @param parameter The parameter that the user specified. * @param parameter The parameter that the user specified.
* @return True if the parameter is valid; otherwise, false. * @return True if the parameter is valid; otherwise, false.
*/ */
virtual bool CanonicalizeParam(LocalUser* user, Channel* channel, std::string& parameter); virtual bool ValidateParam(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);
/** @copydoc ModeHandler::OnParameterMissing */ /** @copydoc ModeHandler::OnParameterMissing */
void OnParameterMissing(User* user, User* dest, Channel* channel) override; void OnParameterMissing(User* user, User* dest, Channel* channel) override;

View File

@ -114,7 +114,7 @@ private:
ExtBan::ManagerRef extbanmgr; ExtBan::ManagerRef extbanmgr;
public: public:
ModeChannelBan(Module* Creator); 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 class ModeChannelKey final

View File

@ -27,7 +27,7 @@ ModeChannelBan::ModeChannelBan(Module* Creator)
syntax = "<mask>"; syntax = "<mask>";
} }
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)) if (!extbanmgr || !extbanmgr->Canonicalize(parameter))
ModeParser::CleanMask(parameter); ModeParser::CleanMask(parameter);

View File

@ -157,7 +157,7 @@ ModeAction ListModeBase::OnModeChange(User* source, User*, Channel* channel, Mod
{ {
// Try to canonicalise the parameter locally. // Try to canonicalise the parameter locally.
LocalUser* lsource = IS_LOCAL(source); LocalUser* lsource = IS_LOCAL(source);
if (lsource && !CanonicalizeParam(lsource, channel, change.param)) if (lsource && !ValidateParam(lsource, channel, change.param))
return MODEACTION_DENY; return MODEACTION_DENY;
// If there was no list // If there was no list
@ -188,27 +188,8 @@ ModeAction ListModeBase::OnModeChange(User* source, User*, Channel* channel, Mod
return MODEACTION_DENY; return MODEACTION_DENY;
} }
/* Ok, it *could* be allowed, now give someone subclassing us cd->list.emplace_back(change.param, change.set_by.value_or(source->nick), change.set_at.value_or(ServerInstance->Time()));
* a chance to validate the parameter. return MODEACTION_ALLOW;
* 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;
}
} }
else else
{ {
@ -231,12 +212,7 @@ ModeAction ListModeBase::OnModeChange(User* source, User*, Channel* channel, Mod
} }
} }
bool ListModeBase::CanonicalizeParam(LocalUser* user, Channel* channel, std::string& parameter) bool ListModeBase::ValidateParam(LocalUser* user, Channel* channel, std::string& parameter)
{
return true;
}
bool ListModeBase::ValidateParam(User* user, Channel* channel, const std::string& parameter)
{ {
return true; return true;
} }

View File

@ -50,7 +50,7 @@ public:
syntax = "<mask>"; syntax = "<mask>";
} }
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)) if (!extbanmgr || !extbanmgr->Canonicalize(parameter))
ModeParser::CleanMask(parameter); ModeParser::CleanMask(parameter);

View File

@ -47,8 +47,9 @@ public:
syntax = "<pattern>"; syntax = "<pattern>";
} }
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) if (parameter.length() > maxlen)
{ {
user->WriteNumeric(Numerics::InvalidModeParameter(chan, this, parameter, "Entry is too long for the spamfilter list.")); user->WriteNumeric(Numerics::InvalidModeParameter(chan, this, parameter, "Entry is too long for the spamfilter list."));

View File

@ -81,8 +81,9 @@ public:
return MOD_RES_DENY; 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 restriction;
std::string prefix; std::string prefix;
if (!ParseEntry(parameter, restriction, prefix)) if (!ParseEntry(parameter, restriction, prefix))

View File

@ -48,7 +48,7 @@ public:
syntax = "<mask>"; syntax = "<mask>";
} }
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)) if (!extbanmgr || !extbanmgr->Canonicalize(parameter))
ModeParser::CleanMask(parameter); ModeParser::CleanMask(parameter);