From 7013ebce8f2bc856983cc97a188376b72e7cd89c Mon Sep 17 00:00:00 2001 From: Michael Poole Date: Mon, 9 Feb 2009 01:30:13 +0000 Subject: [PATCH] Author: Michael Poole Description: Clean up key cleaning: Do not reject non-ASCII characters, and allow ':' after the first character in the key. Add a regression test script to go along with these changes. git-svn-id: file:///home/klmitch/undernet-ircu/undernet-ircu-svn/ircu2/branches/u2_10_12_branch@1905 c9e4aea6-c8fd-4c43-8297-357d70d61c8c --- ChangeLog | 15 +++++++++ include/numeric.h | 1 + ircd/channel.c | 70 +++++++++++++++++++++++++----------------- ircd/s_err.c | 2 +- tests/channel-keys.cmd | 45 +++++++++++++++++++++++++++ 5 files changed, 104 insertions(+), 29 deletions(-) create mode 100644 tests/channel-keys.cmd diff --git a/ChangeLog b/ChangeLog index ec13767..d74896c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2009-02-08 Michael Poole + + * include/numeric.h (ERR_INVALIDKEY): Define new numeric. + + * ircd/s_err.c (ERR_INVALIDKEY): Give it a text string. + + * ircd/channel.c (is_clean_key): Rename from clean_key(), and make + this function responsible for sending error messages to the client + when necessary. + (mode_parse_key): Update to match the new is_clean_key() behavior. + (mode_parse_upass): Likewise. + (mode_parse_apass): Likewise. + + * tests/channel-keys.cmd: New file for regression testing. + 2009-02-08 Michael Poole * include/gline.h (gline_forward_deactivation): Undeclare. diff --git a/include/numeric.h b/include/numeric.h index 48df5b2..bb18601 100644 --- a/include/numeric.h +++ b/include/numeric.h @@ -456,6 +456,7 @@ extern const struct Numeric* get_error_numeric(int err); ERR_WHOSYNTAX 522 dalnet ERR_WHOLIMEXCEED 523 dalnet */ #define ERR_QUARANTINED 524 /* Undernet extension -Vampire */ +#define ERR_INVALIDKEY 525 /* Undernet extension */ #define ERR_NOTLOWEROPLEVEL 560 /* Undernet extension */ #define ERR_NOTMANAGER 561 /* Undernet extension */ diff --git a/ircd/channel.c b/ircd/channel.c index 8ed3eb9..04d28a2 100644 --- a/ircd/channel.c +++ b/ircd/channel.c @@ -2330,15 +2330,41 @@ mode_parse_limit(struct ParseState *state, int *flag_p) } } -/** Helper function to clean key-like parameters. */ -static void -clean_key(char *s) +/** Helper function to validate key-like parameters. + * + * @param[in] state Parse state for feedback to user. + * @param[in] s Key to validate. + * @param[in] command String to pass for need_more_params() command. + * @return Zero on an invalid key, non-zero if the key was okay. + */ +static int +is_clean_key(struct ParseState *state, char *s, char *command) { - int t_len = KEYLEN; + int ii; - while (*s > ' ' && *s != ':' && *s != ',' && t_len--) - s++; - *s = '\0'; + if (s[0] == '\0') { + if (MyUser(state->sptr)) + need_more_params(state->sptr, command); + return 0; + } + else if (s[0] == ':') { + if (MyUser(state->sptr)) + send_reply(state->sptr, ERR_INVALIDKEY, state->chptr->chname); + return 0; + } + for (ii = 0; (ii <= KEYLEN) && (s[ii] != '\0'); ++ii) { + if ((unsigned char)s[ii] <= ' ' || s[ii] == ',') { + if (MyUser(state->sptr)) + send_reply(state->sptr, ERR_INVALIDKEY, state->chptr->chname); + return 0; + } + } + if (ii > KEYLEN) { + if (MyUser(state->sptr)) + send_reply(state->sptr, ERR_INVALIDKEY, state->chptr->chname); + return 0; + } + return 1; } /* @@ -2383,14 +2409,10 @@ mode_parse_key(struct ParseState *state, int *flag_p) state->done |= DONE_KEY_DEL; } - /* clean up the key string */ - clean_key(t_str); - if (!*t_str || *t_str == ':') { /* warn if empty */ - if (MyUser(state->sptr)) - need_more_params(state->sptr, state->dir == MODE_ADD ? "MODE +k" : - "MODE -k"); + /* If the key is invalid, tell the user and bail. */ + if (!is_clean_key(state, t_str, state->dir == MODE_ADD ? "MODE +k" : + "MODE -k")) return; - } if (!state->mbuf) return; @@ -2495,14 +2517,10 @@ mode_parse_upass(struct ParseState *state, int *flag_p) state->done |= DONE_UPASS_DEL; } - /* clean up the upass string */ - clean_key(t_str); - if (!*t_str || *t_str == ':') { /* warn if empty */ - if (MyUser(state->sptr)) - need_more_params(state->sptr, state->dir == MODE_ADD ? "MODE +U" : - "MODE -U"); + /* If the Upass is invalid, tell the user and bail. */ + if (!is_clean_key(state, t_str, state->dir == MODE_ADD ? "MODE +U" : + "MODE -U")) return; - } if (!state->mbuf) return; @@ -2642,14 +2660,10 @@ mode_parse_apass(struct ParseState *state, int *flag_p) state->done |= DONE_APASS_DEL; } - /* clean up the apass string */ - clean_key(t_str); - if (!*t_str || *t_str == ':') { /* warn if empty */ - if (MyUser(state->sptr)) - need_more_params(state->sptr, state->dir == MODE_ADD ? "MODE +A" : - "MODE -A"); + /* If the Apass is invalid, tell the user and bail. */ + if (!is_clean_key(state, t_str, state->dir == MODE_ADD ? "MODE +A" : + "MODE -A")) return; - } if (!state->mbuf) return; diff --git a/ircd/s_err.c b/ircd/s_err.c index b86224f..b05affb 100644 --- a/ircd/s_err.c +++ b/ircd/s_err.c @@ -1082,7 +1082,7 @@ static Numeric replyTable[] = { /* 524 */ { ERR_QUARANTINED, "%s :Channel is quarantined : %s", "524" }, /* 525 */ - { 0 }, + { ERR_INVALIDKEY, "%s :Key is not well-formed", "525" }, /* 526 */ { 0 }, /* 527 */ diff --git a/tests/channel-keys.cmd b/tests/channel-keys.cmd new file mode 100644 index 0000000..c17bb80 --- /dev/null +++ b/tests/channel-keys.cmd @@ -0,0 +1,45 @@ +define srv1 localhost:7601 +define srv1-name irc.example.net +define srv2 localhost:7611 +define srv2-name irc-2.example.net +define cl1-nick Op3rm4n +define cl2-nick Monitor +define channel #keytest + +# Connect a client to each server, and join them to the same channel. +connect cl1 %cl1-nick% oper %srv1% :Some Channel Operator +connect cl2 %cl2-nick% oper %srv2% :Snoopy +:cl1 join %channel% +sync cl1,cl2 +:cl2 join %channel% +sync cl1,cl2 + +# Set a plain and simple key initially. +:cl1 mode %channel% +k foo +:cl2 expect %cl1-nick% mode %channel% \\+k foo + +# Slighly funny quoting here: one : for test-driver.pl and one for quoting. +# The final : makes the key invalid. +:cl1 mode %channel% -k+k foo :::badkey +:cl1 expect %srv1-name% 525 %channel% :Key is not well-formed +:cl2 expect %cl1-nick% mode %channel% -k foo + +# Non-ASCII characters should be accepted in the key, and colons after the first character. +:cl1 mode %channel% +k mötör:head +:cl2 expect %cl1-nick% mode %channel% \\+k mötör:head + +# We need to have a key, too. +:cl1 mode %channel% -k+k mötör:head +:cl1 expect %srv1-name% 461 MODE \\+k :Not enough parameters + +# Are spaces accepted anywhere in the key? +:cl1 mode %channel% +k :: spaced key +:cl1 expect %srv1-name% 525 %channel% :Key is not well-formed + +# What about commas? +:cl1 mode %channel% +k foo,bar +:cl1 expect %srv1-name% 525 %channel% :Key is not well-formed + +# Is the key too long? +:cl1 mode %channel% +k 123456789012345678901234567890 +:cl1 expect %srv1-name% 525 %channel% :Key is not well-formed -- 2.20.1