From bce03de4bca4e3f10112377a86aaf7a7a2d5cfce Mon Sep 17 00:00:00 2001 From: Aaron Jones Date: Sun, 24 Mar 2024 06:45:03 +0000 Subject: [PATCH] modules/operserv/mode: check whether argument adds or removes modes channel_mode() was written in a fragile manner such that it assumes its input is from an IRCd and is trustworthy and well-formed. It is also not capable of reporting an error condition back to its caller. Ordinarily this would be okay; we have to trust our uplink after all, and if it's feeding us malicious protocol data, all hope of maintaining a sensible state of the network is already lost. However, the OS MODE module allows humans to feed arbitrary data into it, and if you forget to prefix that data with + or -, the results can vary from unintentional removal of channel modes [1] to a desync [2]. This is because channel_mode() assumes that if the mode direction is not ADD, it must be DEL; and it looks for a + or - to switch it into ADD or DEL state but starts out in neither of these states. There's nothing we can do in the short term to protect services and the network against malicious operators, but we can protect operators from making this trivial data entry mistake. [1] Executing e.g. "/OS MODE #foo o bar" will (instead of opping bar) deop bar, whether or not bar is opped. This will not corrupt any channel state but is unintuitive and undesirable behaviour. [2] Executing e.g. "/OS MODE #foo i" will convince services that the channel is no longer cmode +i but will not send out a MODE -i to the network because modestack_add_simple() will ignore (and complain about) an invalid mode direction. Reported-By: glguy (Libera.Chat) --- modules/operserv/mode.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/modules/operserv/mode.c b/modules/operserv/mode.c index 77bbec1df8..7fde013813 100644 --- a/modules/operserv/mode.c +++ b/modules/operserv/mode.c @@ -32,13 +32,20 @@ os_cmd_mode(struct sourceinfo *si, int parc, char *parv[]) return; } + modeparc = sjtoken(mode, ' ', modeparv); + + if (modeparc == 0 || (modeparv[0][0] != '+' && modeparv[0][0] != '-')) + { + command_fail(si, fault_badparams, _("The mode parameter(s) given must be to add or " + "remove channel modes.")); + return; + } + wallops("\2%s\2 is using MODE on \2%s\2 (set: \2%s\2)", get_oper_name(si), channel, mode); logcommand(si, CMDLOG_ADMIN, "MODE: \2%s\2 on \2%s\2", mode, channel); command_success_nodata(si, _("Setting modes \2%s\2 on \2%s\2."), mode, channel); - modeparc = sjtoken(mode, ' ', modeparv); - channel_mode(si->service->me, c, modeparc, modeparv); }