Skip to content

Commit

Permalink
modules/operserv/mode: check whether argument adds or removes modes
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
aaronmdjones committed Mar 24, 2024
1 parent 3c9ce6a commit bce03de
Showing 1 changed file with 9 additions and 2 deletions.
11 changes: 9 additions & 2 deletions modules/operserv/mode.c
Expand Up @@ -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);
}

Expand Down

0 comments on commit bce03de

Please sign in to comment.