From 22dec96f9a5dcf5edb0cb177dfed963b53b8bf62 Mon Sep 17 00:00:00 2001 From: Bram Matthys Date: Wed, 9 Oct 2019 17:55:23 +0200 Subject: [PATCH] No clean_channelname() anymore but a valid_channelname() function. Also, deal with servers with different set::allowed-channelchars settings: * We reject the link if set::allowed-channelchars settings differ between UnrealIRCd 5 servers. * For the case where you have a mixed network consisting of UnrealIRCd 4.x and UnrealIRCd 5.x servers we try not to desync, BUT will not allow anyone to join the invalid channels locally. For IRCOps a message is printed with additional information on such a failed JOIN attempt. See https://www.unrealircd.org/docs/Set_block#set::allowed-channelchars for the different settings, which are best and U4<->U5 advice. --- include/h.h | 3 +- src/channel.c | 75 ++++++++++++++++++++++----------- src/conf.c | 5 +-- src/modules/chanmodes/link.c | 31 +++----------- src/modules/extbans/inchannel.c | 6 +-- src/modules/invite.c | 7 ++- src/modules/join.c | 67 +++++++++++++++++------------ src/modules/knock.c | 11 ++--- src/modules/list.c | 4 ++ src/modules/mode.c | 7 ++- src/modules/sajoin.c | 12 ++++-- 11 files changed, 128 insertions(+), 100 deletions(-) diff --git a/include/h.h b/include/h.h index 935a2f693..8b54f3752 100644 --- a/include/h.h +++ b/include/h.h @@ -178,7 +178,7 @@ extern int check_tkls(Client *cptr); /* for services */ extern void del_invite(Client *, Channel *); extern void send_user_joins(Client *, Client *); -extern void clean_channelname(char *); +extern int valid_channelname(const char *); extern long get_access(Client *, Channel *); extern int ban_check_mask(Client *, Channel *, char *, int, char **, char **, int); extern int extban_is_ok_nuh_extban(Client *, Channel *, char *, int, int, int); @@ -949,3 +949,4 @@ extern void utf8_test(void); extern MODVAR int non_utf8_nick_chars_in_use; extern void short_motd(Client *client); extern int should_show_connect_info(Client *client); +extern void send_invalid_channelname(Client *client, char *channelname); diff --git a/src/channel.c b/src/channel.c index e3469345c..3880d2e1c 100644 --- a/src/channel.c +++ b/src/channel.c @@ -62,7 +62,6 @@ char cmodestring[512]; char *clean_ban_mask(char *, int, Client *); void channel_modes(Client *client, char *mbuf, char *pbuf, size_t mbuf_size, size_t pbuf_size, Channel *channel); int sub1_from_channel(Channel *); -void clean_channelname(char *); void del_invite(Client *, Channel *); inline int op_can_override(char* acl, Client *client,Channel *channel,void* extra) @@ -866,54 +865,52 @@ int find_invex(Channel *channel, Client *client) * You must call this before creating a new channel, * eg in case of /JOIN. */ -void clean_channelname(char *cname) +int valid_channelname(const char *cname) { - char *ch = cname; - char *end; + const char *p; + + /* Channel name must start with a dash */ + if (*cname != '#') + return 0; + + if (strlen(cname) > CHANNELLEN) + return 0; if (iConf.allowed_channelchars == ALLOWED_CHANNELCHARS_ANY) { /* The default up to and including UnrealIRCd 4 */ - for (; *ch; ch++) + for (p = cname; *p; p++) { - if (*ch < 33 || *ch == ',' || *ch == ':') - { - *ch = '\0'; - break; - } + if (*p < 33 || *p == ',' || *p == ':') + return 0; } } else if (iConf.allowed_channelchars == ALLOWED_CHANNELCHARS_ASCII) { /* The strict setting: only allow ASCII 32-128, except some chars */ - for (; *ch; ch++) + for (p = cname; *p; p++) { - if (*ch < 33 || *ch == ',' || *ch == ':' || *ch > 127) - { - *ch = '\0'; - break; - } + if (*p < 33 || *p == ',' || *p == ':' || *p > 127) + return 0; } } else if (iConf.allowed_channelchars == ALLOWED_CHANNELCHARS_UTF8) { /* Only allow UTF8, and also disallow some chars */ - for (; *ch; ch++) + for (p = cname; *p; p++) { - if (*ch < 33 || *ch == ',' || *ch == ':') - { - *ch = '\0'; - break; - } + if (*p < 33 || *p == ',' || *p == ':') + return 0; } /* And run it through the UTF8 validator */ - if (!unrl_utf8_validate(cname, (const char **)&end)) - *end = '\0'; + if (!unrl_utf8_validate(cname, (const char **)&p)) + return 0; } else { /* Impossible */ abort(); } + return 1; /* Valid */ } /* @@ -1366,3 +1363,33 @@ int invisible_user_in_channel(Client *target, Channel *channel) return 0; } + +/** Send a message to the user that (s)he is using an invalid channel name. + * This is usually called after an if (MyUser(client) && !valid_channelname(name)). + * @param client The client to send the message to. + * @param channelname The (invalid) channel that the user tried to join. + */ +void send_invalid_channelname(Client *client, char *channelname) +{ + char *reason; + + if (strlen(channelname) > CHANNELLEN) + { + reason = "Channel name is too long"; + } else { + switch(iConf.allowed_channelchars) + { + case ALLOWED_CHANNELCHARS_ASCII: + reason = "Channel name contains illegal characters (must be ASCII)"; + break; + case ALLOWED_CHANNELCHARS_UTF8: + reason = "Channel name contains illegal characters (must be valid UTF8)"; + break; + case ALLOWED_CHANNELCHARS_ANY: + default: + reason = "Channel name contains illegal characters"; + } + } + + sendnumeric(client, ERR_FORBIDDENCHANNEL, channelname, reason); +} diff --git a/src/conf.c b/src/conf.c index 41a060f6c..180b47767 100644 --- a/src/conf.c +++ b/src/conf.c @@ -9090,7 +9090,6 @@ int _test_offchans(ConfigFile *conf, ConfigEntry *ce) { int errors = 0; ConfigEntry *cep, *cep2; - char checkchan[CHANNELLEN + 1]; if (!ce->ce_entries) { @@ -9107,9 +9106,7 @@ int _test_offchans(ConfigFile *conf, ConfigEntry *ce) errors++; continue; } - strcpy(checkchan, cep->ce_varname); /* safe */ - clean_channelname(checkchan); - if (strcmp(checkchan, cep->ce_varname) || (*cep->ce_varname != '#')) + if (!valid_channelname(cep->ce_varname)) { config_error("%s:%i: official-channels: '%s' is not a valid channel name.", cep->ce_fileptr->cf_filename, cep->ce_varlinenum, cep->ce_varname); diff --git a/src/modules/chanmodes/link.c b/src/modules/chanmodes/link.c index 622654fa6..ac16ba38a 100644 --- a/src/modules/chanmodes/link.c +++ b/src/modules/chanmodes/link.c @@ -120,23 +120,16 @@ int cmodeL_is_ok(Client *client, Channel *channel, char mode, char *para, int ty if (type == EXCHK_PARAM) { /* Check parameter.. syntax is +L #channel */ - char buf[CHANNELLEN+1], *p; - if (strchr(para, ',')) return EX_DENY; /* multiple channels not permitted */ - if (!IsChannelName(para)) + if (!valid_channelname(para)) { if (MyUser(client)) sendnumeric(client, ERR_NOSUCHCHANNEL, para); return EX_DENY; } - /* This needs to be exactly the same as in conv_param... */ - strlcpy(buf, para, sizeof(buf)); - clean_channelname(buf); - if ((p = strchr(buf, ':'))) - *p = '\0'; - if (find_channel(buf, NULL) == channel) + if (find_channel(para, NULL) == channel) { if (MyUser(client)) sendnumeric(client, ERR_CANNOTCHANGECHANMODE, 'L', @@ -178,20 +171,14 @@ char *cmodeL_get_param(void *r_in) /** Convert parameter to something proper. * NOTE: client may be NULL */ -char *cmodeL_conv_param(char *param_in, Client *client) +char *cmodeL_conv_param(char *param, Client *client) { - static char buf[CHANNELLEN+1]; char *p; - strlcpy(buf, param_in, sizeof(buf)); - clean_channelname(buf); - if ((p = strchr(buf, ':'))) - *p = '\0'; + if (!valid_channelname(param)) + return NULL; - if (*buf == '\0') - strcpy(buf, "#"); /* better safe than sorry */ - - return buf; + return param; } void cmodeL_free_param(void *r) @@ -287,13 +274,9 @@ char *extban_link_conv_param(char *param) return NULL; *matchby++ = '\0'; - if (*chan != '#' || strchr(param, ',')) + if (!valid_channelname(chan)) return NULL; - if (strlen(chan) > CHANNELLEN) - chan[CHANNELLEN] = '\0'; - clean_channelname(chan); - // Possibly stack multiple extbans, this is a little convoluted due to extban API limitations snprintf(tmpmask, sizeof(tmpmask), "~?:%s", matchby); newmask = extban_conv_param_nuh_or_extban(tmpmask); diff --git a/src/modules/extbans/inchannel.c b/src/modules/extbans/inchannel.c index 58990ea94..ab5098c9d 100644 --- a/src/modules/extbans/inchannel.c +++ b/src/modules/extbans/inchannel.c @@ -80,10 +80,8 @@ char *extban_inchannel_conv_param(char *para) if ((*chan != '#') && (*chan != '*') && (*chan != '?')) return NULL; - if (strlen(chan) > CHANNELLEN) - chan[CHANNELLEN] = '\0'; - - clean_channelname(chan); + if (!valid_channelname(chan)) + return NULL; p = strchr(chan, ':'); /* ~r:#chan:*.blah.net is not allowed (for now) */ if (p) diff --git a/src/modules/invite.c b/src/modules/invite.c index 44e2d6150..a2e94cba6 100644 --- a/src/modules/invite.c +++ b/src/modules/invite.c @@ -93,8 +93,11 @@ CMD_FUNC(cmd_invite) return; } - if (MyConnect(client)) - clean_channelname(parv[2]); + if (MyConnect(client) && !valid_channelname(parv[2])) + { + sendnumeric(client, ERR_NOSUCHCHANNEL, parv[2]); + return; + } if (!(channel = find_channel(parv[2], NULL))) { diff --git a/src/modules/join.c b/src/modules/join.c index 5a1e67602..f8f5d0760 100644 --- a/src/modules/join.c +++ b/src/modules/join.c @@ -360,11 +360,11 @@ void _join_channel(Channel *channel, Client *client, MessageTag *recv_mtags, int */ void _do_join(Client *client, int parc, char *parv[]) { - char jbuf[BUFSIZE]; + char jbuf[BUFSIZE], jbuf2[BUFSIZE]; Membership *lp; Channel *channel; char *name, *key = NULL; - int i, flags = 0, ishold; + int i, flags = 0, ishold; char *p = NULL, *p2 = NULL; TKL *tklban; int ntargets = 0; @@ -394,30 +394,40 @@ void _do_join(Client *client, int parc, char *parv[]) ** Rebuild list of channels joined to be the actual result of the ** JOIN. Note that "JOIN 0" is the destructive problem. */ - for (i = 0, name = strtoken(&p, parv[1], ","); name; - name = strtoken(&p, NULL, ",")) + for (i = 0, name = strtoken(&p, parv[1], ","); + name; + i++, name = strtoken(&p, NULL, ",")) { if (MyUser(client) && (++ntargets > maxtargets)) { sendnumeric(client, ERR_TOOMANYTARGETS, name, maxtargets, "JOIN"); break; } - /* pathological case only on longest channel name. - ** If not dealt with here, causes desynced channel ops - ** since ChannelExists() doesn't see the same channel - ** as one being joined. cute bug. Oct 11 1997, Dianora/comstud - ** Copied from Dianora's "hybrid 5" ircd. - */ - - if (strlen(name) > CHANNELLEN) /* same thing is done in get_channel() */ - name[CHANNELLEN] = '\0'; - - if (MyConnect(client)) - clean_channelname(name); if (*name == '0' && !atoi(name)) { - (void)strcpy(jbuf, "0"); - i = 1; + /* UnrealIRCd 5: we only support "JOIN 0", + * "JOIN 0,#somechan" etc... so only at the beginning. + * We do not support it half-way like "JOIN #a,0,#b" + * since that doesn't make sense, unless you are flooding... + * We still support it in remote joins for compatibility. + */ + if (MyUser(client) && (i != 0)) + continue; + strlcpy(jbuf, "0", sizeof(jbuf)); + continue; + } else + if (MyConnect(client) && !valid_channelname(name)) + { + send_invalid_channelname(client, name); + if (IsOper(client) && find_channel(name, NULL)) + { + /* Give IRCOps a bit more information */ + sendnotice(client, "Channel '%s' is unjoinable because it contains illegal characters. " + "However, it does exist because another server in your " + "network, which has a more loose restriction, created it. " + "See https://www.unrealircd.org/docs/Set_block#set::allowed-channelchars", + name); + } continue; } else if (!IsChannelName(name)) @@ -427,22 +437,25 @@ void _do_join(Client *client, int parc, char *parv[]) continue; } if (*jbuf) - (void)strlcat(jbuf, ",", sizeof jbuf); - (void)strlncat(jbuf, name, sizeof jbuf, sizeof(jbuf) - i - 1); - i += strlen(name) + 1; + strlcat(jbuf, ",", sizeof jbuf); + strlcat(jbuf, name, sizeof(jbuf)); } - /* This strcpy should be safe since jbuf contains the "filtered" - * result of parv[1] which should never be larger than the source. + + /* We are going to overwrite 'jbuf' with the calls to strtoken() + * a few lines further down. Copy it to 'jbuf2' and make that + * the new parv[1]. */ - (void)strcpy(parv[1], jbuf); + strlcpy(jbuf2, jbuf, sizeof(jbuf2)); + parv[1] = jbuf2; p = NULL; if (parv[2]) key = strtoken(&p2, parv[2], ","); parv[2] = NULL; /* for cmd_names call later, parv[parc] must == NULL */ - for (name = strtoken(&p, jbuf, ","); name; - key = (key) ? strtoken(&p2, NULL, ",") : NULL, - name = strtoken(&p, NULL, ",")) + + for (name = strtoken(&p, jbuf, ","); + name; + key = key ? strtoken(&p2, NULL, ",") : NULL, name = strtoken(&p, NULL, ",")) { MessageTag *mtags = NULL; diff --git a/src/modules/knock.c b/src/modules/knock.c index 122e4501e..7c69e51c0 100644 --- a/src/modules/knock.c +++ b/src/modules/knock.c @@ -79,17 +79,12 @@ CMD_FUNC(cmd_knock) return; } - if (MyConnect(client)) - clean_channelname(parv[1]); - - /* bugfix for /knock PRv Please? */ - if (*parv[1] != '#') + if (MyConnect(client) && !valid_channelname(parv[1])) { - sendnumeric(client, ERR_CANNOTKNOCK, - parv[1], "Remember to use a # prefix in channel name"); - + sendnumeric(client, ERR_NOSUCHCHANNEL, parv[1]); return; } + if (!(channel = find_channel(parv[1], NULL))) { sendnumeric(client, ERR_CANNOTKNOCK, parv[1], "Channel does not exist!"); diff --git a/src/modules/list.c b/src/modules/list.c index dc8e6e3a2..57f8d640e 100644 --- a/src/modules/list.c +++ b/src/modules/list.c @@ -369,6 +369,10 @@ int send_list(Client *client) if (!IsOper(client) && iConf.hide_list && Find_channel_allowed(client, channel->chname)) continue; + /* Similarly, hide unjoinable channels for non-ircops since it would be confusing */ + if (!IsOper(client) && !valid_channelname(channel->chname)) + continue; + /* Much more readable like this -- codemastr */ if ((!lopt->showall)) { diff --git a/src/modules/mode.c b/src/modules/mode.c index b7e264010..98e9d381b 100644 --- a/src/modules/mode.c +++ b/src/modules/mode.c @@ -120,8 +120,11 @@ CMD_FUNC(cmd_mode) return; } - if (MyConnect(client)) - clean_channelname(parv[1]); + if (MyConnect(client) && !valid_channelname(parv[1])) + { + sendnumeric(client, ERR_NOSUCHCHANNEL, parv[1]); + return; + } if (parc < 3) { diff --git a/src/modules/sajoin.c b/src/modules/sajoin.c index f15b3a7a9..4a526ec7f 100644 --- a/src/modules/sajoin.c +++ b/src/modules/sajoin.c @@ -157,8 +157,11 @@ CMD_FUNC(cmd_sajoin) } if (strlen(name) > CHANNELLEN) - name[CHANNELLEN] = 0; - clean_channelname(name); + { + sendnotice(client, "Channel name too long: %s", name); + continue; + } + if (*name == '0' && !atoi(name) && !sjmode) { (void)strcpy(jbuf, "0"); @@ -166,9 +169,10 @@ CMD_FUNC(cmd_sajoin) parted = 1; continue; } - if (*name == '0' || !IsChannelName(name)) + + if (!valid_channelname(name)) { - sendnumeric(client, ERR_NOSUCHCHANNEL, name); + send_invalid_channelname(client, name); continue; }