From 020588394174c23220f3a80f715a36f104e83f1a Mon Sep 17 00:00:00 2001 From: Jonas Rebmann Date: Sat, 13 Dec 2025 18:09:23 +0100 Subject: [PATCH] telnet.c: rewrite command handling The previous implementation was unable to handle any command sequences split across the (maximum) 1024 bytes buffers, leading to crashes, out-of-bounds access and data corruption. The previous implementation went into great detail handling plenty of command strings but only provided stubs debug-printing those commands. To keep the code concise, the new version prints a hexdump of all command sequences it extracts from the bytestream (regardless of whether they're handled or not). Signed-off-by: Jonas Rebmann --- telnet.c | 506 ++++++++++++++++--------------------------------------- 1 file changed, 143 insertions(+), 363 deletions(-) diff --git a/telnet.c b/telnet.c index 5878a01..95caedf 100644 --- a/telnet.c +++ b/telnet.c @@ -29,6 +29,12 @@ #include "microcom.h" +struct telnet_ios { + struct ios_ops base; + unsigned char iac_buf[8]; + ssize_t iac_buf_len; +}; + static int telnet_printf(struct ios_ops *ios, const char *format, ...) { char buf[20]; @@ -60,349 +66,12 @@ static int telnet_printf(struct ios_ops *ios, const char *format, ...) return written; } -static ssize_t get(unsigned char *buf, unsigned char *out, size_t len) -{ - if (!len) - return -1; - - if (buf[0] == IAC) { - if (len < 1) - return -1; - if (buf[1] == IAC) { - *out = IAC; - return 2; - } - return -1; - } else { - *out = buf[0]; - return 1; - } -} - -static size_t getl(unsigned char *buf, uint32_t *out, size_t len) -{ - *out = 0; - int i; - size_t offset = 0; - - for (i = 0; i < 4; ++i) { - ssize_t getres; - unsigned char c; - - getres = get(buf + offset, &c, len - offset); - if (getres < 0) - return getres; - - *out <<= 8; - *out |= c; - - offset += getres; - } - - return offset; -} -/* This is called with buf[-2:0] being IAC SB COM_PORT_OPTION */ -static int do_com_port_option(struct ios_ops *ios, unsigned char *buf, int len) -{ - int i = 2; - - switch (buf[1]) { - case SET_BAUDRATE_CS: - dbg_printf("SET_BAUDRATE_CS "); - break; - case SET_DATASIZE_CS: - dbg_printf("SET_DATASIZE_CS "); - break; - case SET_PARITY_CS: - dbg_printf("SET_PARITY_CS "); - break; - case SET_STOPSIZE_CS: - dbg_printf("SET_STOPSIZE_CS "); - break; - case SET_CONTROL_CS: - dbg_printf("SET_CONTROL_CS "); - break; - case NOTIFY_LINESTATE_CS: - dbg_printf("NOTIFY_LINESTATE_CS "); - break; - case NOTIFY_MODEMSTATE_CS: - dbg_printf("NOTIFY_MODEMSTATE_CS "); - break; - case FLOWCONTROL_SUSPEND_CS: - dbg_printf("FLOWCONTROL_SUSPEND_CS "); - break; - case FLOWCONTROL_RESUME_CS: - dbg_printf("FLOWCONTROL_RESUME_CS "); - break; - case SET_LINESTATE_MASK_CS: - dbg_printf("SET_LINESTATE_MASK_CS "); - break; - case SET_MODEMSTATE_MASK_CS: - dbg_printf("SET_MODEMSTATE_MASK_CS "); - break; - case PURGE_DATA_CS: - dbg_printf("PURGE_DATA_CS "); - break; - case SET_BAUDRATE_SC: - { - uint32_t baudrate; - ssize_t getres = getl(buf + 2, &baudrate, len - 2); - - if (getres < 0) { - fprintf(stderr, "Incomplete or broken SB (SET_BAUDRATE_SC)\n"); - return getres; - } - dbg_printf("SET_BAUDRATE_SC %u ", baudrate); - i += getres;; - } - break; - case SET_DATASIZE_SC: - dbg_printf("SET_DATASIZE_SC "); - break; - case SET_PARITY_SC: - dbg_printf("SET_PARITY_SC "); - break; - case SET_STOPSIZE_SC: - dbg_printf("SET_STOPSIZE_SC "); - break; - case SET_CONTROL_SC: - { - unsigned char ctrl; - ssize_t getres = get(buf + 2, &ctrl, len - 2); - - if (getres < 0) { - fprintf(stderr, "Incomplete or broken SB (SET_CONTROL_SC)\n"); - return getres; - } - - dbg_printf("SET_CONTROL_SC 0x%02x ", ctrl); - i += getres; - } - break; - case NOTIFY_LINESTATE_SC: - dbg_printf("NOTIFY_LINESTATE_SC "); - break; - case NOTIFY_MODEMSTATE_SC: - { - unsigned char ms; - ssize_t getres = get(buf + 2, &ms, len - 2); - - if (getres < 0) { - fprintf(stderr, "Incomplete or broken SB (NOTIFY_MODEMSTATE_SC)\n"); - return getres; - } - - dbg_printf("NOTIFY_MODEMSTATE_SC 0x%02x ", ms); - i += getres; - } - case FLOWCONTROL_SUSPEND_SC: - dbg_printf("FLOWCONTROL_SUSPEND_SC "); - break; - case FLOWCONTROL_RESUME_SC: - dbg_printf("FLOWCONTROL_RESUME_SC "); - break; - case SET_LINESTATE_MASK_SC: - dbg_printf("SET_LINESTATE_MASK_SC "); - break; - case SET_MODEMSTATE_MASK_SC: - dbg_printf("SET_MODEMSTATE_MASK_SC "); - break; - case PURGE_DATA_SC: - dbg_printf("PURGE_DATA_SC "); - break; - default: - dbg_printf("??? %d ", buf[1]); - break; - } - - while (i < len) { - if (buf[i] == IAC) { - if (i + 1 < len && buf[i+1] == IAC) { - /* quoted IAC -> unquote */ - ++i; - } else if (i + 1 < len && buf[i+1] == SE) { - dbg_printf("IAC SE\n"); - return i + 2; - } - } - dbg_printf("%d ", buf[i]); - - ++i; - } - - fprintf(stderr, "Incomplete SB string\n"); - return -EINVAL; -} - -/* This is called with buf[-2:0] being IAC SB COM_PORT_OPTION */ -static int do_binary_transmission_option(struct ios_ops *ios, unsigned char *buf, int len) -{ - /* There are no subcommands for the BINARY_TRANSMISSION option (rfc856) */ - return -EINVAL; -} - -struct telnet_option { - unsigned char id; - const char *name; - int (*subneg_handler)(struct ios_ops *ios, unsigned char *buf, int len); - bool sent_will; -}; - -#define TELNET_OPTION(x) .id = TELNET_OPTION_ ## x, .name = #x - -static const struct telnet_option telnet_options[] = { - { - TELNET_OPTION(COM_PORT_CONTROL), - .subneg_handler = do_com_port_option, - .sent_will = true, - }, { - TELNET_OPTION(BINARY_TRANSMISSION), - .subneg_handler = do_binary_transmission_option, - .sent_will = true, - }, { - TELNET_OPTION(ECHO), - }, { - TELNET_OPTION(SUPPRESS_GO_AHEAD), - } -}; - -static const struct telnet_option *get_telnet_option(unsigned char id) -{ - int i; - - for (i = 0; i < ARRAY_SIZE(telnet_options); ++i) { - if (id == telnet_options[i].id) - return &telnet_options[i]; - } - - return NULL; -} - - -/* This function is called with buf[-2:-1] being IAC SB */ -static int do_subneg(struct ios_ops *ios, unsigned char *buf, int len) -{ - const struct telnet_option *option = get_telnet_option(buf[0]); - - if (option) - dbg_printf("%s ", option->name); - if (option->subneg_handler) { - return option->subneg_handler(ios, buf, len); - } else { - /* skip over subneg string */ - int i; - for (i = 0; i < len - 1; ++i) { - if (buf[i] != IAC) { - dbg_printf("%d ", buf[i]); - continue; - } - - if (buf[i + 1] == SE) { - dbg_printf("IAC SE\n"); - return i + 1; - } - - /* skip over IAC IAC */ - if (buf[i + 1] == IAC) { - dbg_printf("%d \n", IAC); - i++; - } - } - - /* the subneg string isn't finished yet */ - if (i == len - 1) - dbg_printf("%d", buf[i]); - dbg_printf("\\\n"); - fprintf(stderr, "Incomplete SB string\n"); - - return -EINVAL; - } -} - -/* This function is called with buf[0] being IAC. */ -static int handle_command(struct ios_ops *ios, unsigned char *buf, int len) -{ - int ret; - const struct telnet_option *option; - - /* possible out-of-bounds access */ - switch (buf[1]) { - case SB: - dbg_printf("SB "); - ret = do_subneg(ios, &buf[2], len - 2); - if (ret < 0) - return ret; - return ret + 2; - - case WILL: - option = get_telnet_option(buf[2]); - if (option) - dbg_printf("WILL %s", option->name); - else - dbg_printf("WILL #%d", buf[2]); - - if (option && option->subneg_handler) { - /* ok, we already requested that, so take this as - * confirmation to actually do COM_PORT stuff. - * Everything is fine. Don't reconfirm to prevent an - * request/confirm storm. - */ - dbg_printf("\n"); - } else { - /* unknown/unimplemented option -> DONT */ - dbg_printf(" -> DONT\n"); - telnet_printf(ios, "%c%c%c", IAC, DONT, buf[2]); - } - return 3; - - case WONT: - option = get_telnet_option(buf[2]); - if (option) - dbg_printf("WONT %s\n", option->name); - else - dbg_printf("WONT #%d\n", buf[2]); - return 3; - - case DO: - option = get_telnet_option(buf[2]); - if (option) - dbg_printf("DO %s", option->name); - else - dbg_printf("DO #%d", buf[2]); - - if (option && option->sent_will) { - /* - * This is a confirmation of an WILL sent by us before. - * There is nothing to do now. - */ - dbg_printf("\n"); - } else { - /* Oh, cannot handle that one, so send a WONT */ - dbg_printf(" -> WONT\n"); - telnet_printf(ios, "%c%c%c", IAC, WONT, buf[2]); - } - return 3; - - case DONT: - option = get_telnet_option(buf[2]); - if (option) - dbg_printf("DONT %s\n", option->name); - else - dbg_printf("DONT #%d\n", buf[2]); - return 3; - - default: - dbg_printf("??? %d\n", buf[1]); - return 1; - } -} - -static ssize_t telnet_write(struct ios_ops *ios, const void *buf, size_t count) +static ssize_t telnet_write(struct ios_ops *ios, const unsigned char *buf, size_t count) { size_t handled = 0; ssize_t ret; - void *iac; + unsigned char *iac; /* * To send an IAC character in the data stream, two IACs must be sent. @@ -429,40 +98,151 @@ static ssize_t telnet_write(struct ios_ops *ios, const void *buf, size_t count) return ret + handled; } -static ssize_t telnet_read(struct ios_ops *ios, void *buf, size_t count) + +static ssize_t bufskip(unsigned char *buf, ssize_t skip, ssize_t buf_len) +{ + if (debug) { + for (size_t i = 0; i < skip; i++) { + printf("%02X ", buf[i]); + } + printf("\n"); + } + memmove(buf, buf + skip, buf_len - skip); + return skip; +} + +static void telnet_handle_triplet(unsigned char cmd, unsigned char opt) { - ssize_t ret = read(ios->fd, buf, count); - void *iac; - size_t handled = 0; + switch (cmd) { + case DO: + telnet_printf(ios, "%c%c%c", IAC, WONT, opt); + break; + case WILL: + telnet_printf(ios, "%c%c%c", IAC, DONT, opt); + break; + } +} - if (ret <= 0) - return ret; +/** + * process command sequence + * @ios ios struct + * @buf first byte of command sequence (an IAC if this isn't a continuation of an ongoing sequence) + * @buf_len total number of bytes available at buf + * + * Returns: + * the number of bytes that must be stripped from the buffer + * + * Description: + * In order to properly handle sequences received split across buffers, priv->iac_buf stores + * the current partially received section of a command. + * + * Commands as relevant for bytesteam processing: + * - escape IAC,IAC to a literal IAC/255/0xff + * - rfc 854 without option: IAC,[240-250] two byte seq + * - rfc 854 with 1 byte opt: IAC,[251-254] + * - rfc 854/2217 subneg: IAC,SB,...,IAC,SE + */ +static ssize_t handle_iac(struct ios_ops *ios, unsigned char *buf, ssize_t buf_len) +{ + struct telnet_ios *priv = (struct telnet_ios *)ios; + ssize_t consumed = 0; - while ((iac = memchr(buf + handled, IAC, ret - handled)) != NULL) { - handled = iac - buf; + assert(buf_len > 0); - /* XXX: possible out-of-bounds access */ - if (((unsigned char *)iac)[1] == IAC) { - /* duplicated IAC = one payload IAC */ - ret -= 1; - memmove(iac, iac + 1, ret - (iac - buf)); - handled += 1; + if (priv->iac_buf_len == 0) { + if (buf[0] == IAC) { + priv->iac_buf[0] = buf[consumed]; + priv->iac_buf_len++; } else { - int iaclen = handle_command(ios, iac, ret - handled); + return 0; + } + consumed++; + } - if (iaclen < 0) - return iaclen; + if (consumed == buf_len) + return consumed; + + if (priv->iac_buf_len == 1) { + if (buf[consumed] == IAC) { + /* sequence was IAC, IAC, consume only one to produce literal IAC in output */ + priv->iac_buf_len = 0; + return consumed; + } else if (buf[consumed] >= 240 && buf[consumed] <= 249) { + /* two byte sequence: skip it */ + /* TODO: handle 2B telnet */ + priv->iac_buf_len = 0; + return consumed + 1; + } else if (buf[consumed] >= 251 && buf[consumed] <= 254) { + priv->iac_buf[1] = buf[consumed]; + priv->iac_buf_len++; + } else if (buf[consumed] == SB) { + priv->iac_buf[1] = buf[consumed]; + priv->iac_buf_len++; + } else { + /* unknown/invalid sequence, interpret as unknown 2 byte seq */ + priv->iac_buf_len = 0; + return consumed + 1; + } + consumed++; + } - memmove(iac, iac + iaclen, ret - (handled + iaclen)); - ret -= iaclen; + if (consumed == buf_len) + return consumed; + + if (priv->iac_buf[1] >= 251 && priv->iac_buf[1] <= 254) { + /* three byte sequence: skip it */ + telnet_handle_triplet(priv->iac_buf[1], buf[consumed]); + priv->iac_buf_len = 0; + return consumed + 1; + consumed++; + } else if (priv->iac_buf[1] == SB) { + /* we're within a subneg, skip until IAC,SE that isn't preceeded by another IAC */ + /* TODO: handle subneg content */ + for (; consumed < buf_len; consumed++) { + if (buf[consumed] == IAC) { + if (priv->iac_buf_len == 2) { + priv->iac_buf[2] = buf[consumed]; + priv->iac_buf_len++; + } else { + assert(priv->iac_buf[2] == IAC); + assert(priv->iac_buf_len == 3); + priv->iac_buf_len--; + } + } else if (priv->iac_buf_len == 3 && buf[consumed] == SE) { + priv->iac_buf_len = 0; + return consumed; + } } + return consumed; } - if (ret) { + fprintf(stderr, "parse error\n"); + exit(1); + +} + +static ssize_t telnet_read(struct ios_ops *ios, unsigned char *buf, size_t count) +{ + ssize_t ret; + unsigned char *iac; + size_t handled = 0; + + ret = read(ios->fd, buf, count); + + if (ret <= 0) return ret; - } else { - errno = EAGAIN; - return -1; + + /* possibly unfinished command sequence from previous buffer */ + ret -= bufskip(buf, handle_iac(ios, buf, ret), ret); + while (++handled < ret) { + iac = memchr(buf + handled, IAC, ret - handled); + if (iac) { + handled = iac - buf; + ret -= bufskip(iac, handle_iac(ios, iac, ret - handled), ret - handled); + } else { + break; + } } + return ret; } static int telnet_set_speed(struct ios_ops *ios, unsigned long speed) @@ -535,7 +315,7 @@ struct ios_ops *telnet_init(char *hostport) struct ios_ops *ios; char connected_host[256], connected_port[30]; - ios = malloc(sizeof(*ios)); + ios = calloc(1, sizeof(struct telnet_ios)); if (!ios) return NULL;