diff --git a/modules/mod_irc_client.c b/modules/mod_irc_client.c index bb2a112..f4363c3 100644 --- a/modules/mod_irc_client.c +++ b/modules/mod_irc_client.c @@ -341,14 +341,20 @@ static const char *callback_msg_type_name(enum irc_callback_msg_type type) static int msg_relay(struct bbs_irc_client *client, enum relay_flags flags, enum irc_msg_type type, const char *channel, const char *prefix, int ctcp, const char *body, size_t len) { + int res = 0; const char *scope = flags & RELAY_TO_IRC ? flags & RELAY_FROM_IRC ? "to/from" : "to" : "from"; enum irc_callback_msg_type cb_type = callback_msg_type(type); - bbs_debug(7, "Broadcasting %s %s client %s, channel %s, prefix %s, CTCP: %d: %.*s\n", callback_msg_type_name(cb_type), scope, client->name, channel, prefix, ctcp, (int) len, body); + bbs_debug(7, "Broadcasting %s %s client %s, channel %s, prefix %s, CTCP: %d, length %lu: %.*s\n", callback_msg_type_name(cb_type), scope, client->name, channel, prefix, ctcp, len, (int) len, body); + + if (len > 512) { + bbs_warning("%lu-byte message is too long to send to IRC\n", len); + return -1; + } /* Relay the message to everyone */ RWLIST_RDLOCK(&irc_clients); /* XXX Really just need to lock *this* client to prevent it from being removed, not all of them */ if (flags & RELAY_TO_IRC) { - irc_client_msg(client->client, channel, body); /* Actually send to IRC */ + res = irc_client_msg(client->client, channel, body); /* Actually send to IRC */ } if (flags & RELAY_FROM_IRC) { /* Only execute callback on messages received *FROM* IRC client, not messages *TO* it. */ if (client->callbacks) { @@ -363,7 +369,7 @@ static int msg_relay(struct bbs_irc_client *client, enum relay_flags flags, enum } } RWLIST_UNLOCK(&irc_clients); - return 0; + return res; } static void __bot_handler(struct bbs_irc_client *client, enum relay_flags flags, const char *channel, const char *prefix, int ctcp, const char *msg) @@ -640,7 +646,7 @@ static void *client_relay(void *varg) int __attribute__ ((format (gnu_printf, 2, 3))) bbs_irc_client_send(const char *clientname, const char *fmt, ...) { struct bbs_irc_client *client; - char *buf; + char buf[IRC_MAX_MSG_LEN + 1]; int res, len; va_list ap; @@ -660,10 +666,11 @@ int __attribute__ ((format (gnu_printf, 2, 3))) bbs_irc_client_send(const char * } va_start(ap, fmt); - len = vasprintf(&buf, fmt, ap); + len = vsnprintf(buf, sizeof(buf), fmt, ap); va_end(ap); - if (len < 0) { + if (len >= (int) sizeof(buf)) { + bbs_warning("Truncation occured trying to send %d-byte IRC message\n", len); RWLIST_UNLOCK(&irc_clients); return -1; } @@ -672,14 +679,13 @@ int __attribute__ ((format (gnu_printf, 2, 3))) bbs_irc_client_send(const char * res = irc_send(client->client, "%s", buf); RWLIST_UNLOCK(&irc_clients); - free(buf); return res; } int __attribute__ ((format (gnu_printf, 4, 5))) bbs_irc_client_msg(const char *clientname, const char *channel, const char *prefix, const char *fmt, ...) { struct bbs_irc_client *client; - char *buf; + char buf[IRC_MAX_MSG_LEN + 1]; int res, len; va_list ap; @@ -699,10 +705,11 @@ int __attribute__ ((format (gnu_printf, 4, 5))) bbs_irc_client_msg(const char *c } va_start(ap, fmt); - len = vasprintf(&buf, fmt, ap); + len = vsnprintf(buf, sizeof(buf), fmt, ap); va_end(ap); - if (len < 0) { + if (len >= (int) sizeof(buf)) { + bbs_warning("Truncation occured trying to format %d-byte IRC message\n", len); RWLIST_UNLOCK(&irc_clients); return -1; } @@ -720,7 +727,6 @@ int __attribute__ ((format (gnu_printf, 4, 5))) bbs_irc_client_msg(const char *c res = msg_relay(client, RELAY_TO_IRC, IRC_CMD_PRIVMSG, channel, prefix, 0, buf, (size_t) len); /* No prefix */ RWLIST_UNLOCK(&irc_clients); - free(buf); return res; } diff --git a/modules/mod_irc_relay.c b/modules/mod_irc_relay.c index c0a7719..9fb0b7f 100644 --- a/modules/mod_irc_relay.c +++ b/modules/mod_irc_relay.c @@ -725,7 +725,9 @@ static int netirc_cb(const char *channel, const char *sender, const char *msg) if (cp->client2) { if (sender && !cp->ircuser) { /* We're sending to a real IRC channel using a client, so we need to pack the sender name into the message itself, if present. */ if (!ctcp) { - snprintf(fullmsg, sizeof(fullmsg), "<%s> %s", sender, msg); + if (snprintf(fullmsg, sizeof(fullmsg), "<%s> %s", sender, msg) >= (int) sizeof(fullmsg)) { + bbs_warning("Truncation when prefixing sending username to message\n"); + } } msg = fullmsg; } @@ -739,7 +741,9 @@ static int netirc_cb(const char *channel, const char *sender, const char *msg) if (cp->client1) { if (sender && !cp->ircuser) { /* We're sending to a real IRC channel using a client, so we need to pack the sender name into the message itself, if present. */ if (!ctcp) { - snprintf(fullmsg, sizeof(fullmsg), "<%s> %s", sender, msg); + if (snprintf(fullmsg, sizeof(fullmsg), "<%s> %s", sender, msg) >= (int) sizeof(fullmsg)) { + bbs_warning("Truncation when prefixing sending username to message\n"); + } } msg = fullmsg; } diff --git a/nets/net_irc.c b/nets/net_irc.c index 10567c0..c19a89c 100644 --- a/nets/net_irc.c +++ b/nets/net_irc.c @@ -864,46 +864,141 @@ int irc_relay_set_topic(const char *channel, const char *topic) return 0; } -int _irc_relay_send_multiline(const char *channel, enum channel_user_modes modes, const char *relayname, const char *sender, const char *hostsender, const char *msg, int(*transform)(const char *line, char *buf, size_t len), const char *ircuser, void *mod) -{ - char linebuf[512]; - char *dup, *lines; - const char *line; +static int transform_and_send(const char *channel, enum channel_user_modes modes, const char *relayname, const char *sender, const char *hostsender, const char *restrict msg, int(*transform)(const char *line, char *buf, size_t len), const char *ircuser, void *mod) +{ + /* The maximum length of a single IRC message is 512 characters, + * but we use a slightly larger buffer here in case the transform + * function replaces substrings with larger substrings. + * If we limit the buffer to 512, and we get a message of the maximum + * length, expansion will fail since truncation will occur. + * This should be good enough to allow reasonable transformation of + * a message of any valid length. + * If the buffer is still too small, we'll dynamically allocate one. + */ + char linebuf[624]; + size_t linelen; + int res; + /* We use both final and line, so that we can accept a const argument (msg) + * and provide a const argument to _irc_relay_send. + * The added complication avoids the need to allocate if the length + * of the message is sufficiently small, which is the common case. */ + char *line = NULL; + const char *final; + char *dynbuf = NULL; - /*! \todo What if message lines are > 512 characters? */ + if (strlen_zero(msg)) { + return -1; + } - if (!strchr(msg, '\n')) { /* Avoid unnecessarily allocating memory if we don't have to. */ - if (transform) { - if (transform(msg, linebuf, sizeof(linebuf)) < 0) { + if (transform) { + size_t origlen = strlen(msg); + if (origlen >= sizeof(linebuf) - 50) { + /* Stack allocated buffer is probably too small. + * Dynamically allocate. */ + if (origlen >= 4096) { + bbs_warning("Refusing to relay excessively long message of length %lu\n", origlen); return -1; } - line = linebuf; + dynbuf = malloc(origlen + 50); + if (ALLOC_FAILURE(dynbuf)) { + return -1; + } + strcpy(dynbuf, msg); /* Safe */ + res = transform(msg, dynbuf, origlen + 50); } else { - line = msg; + res = transform(msg, linebuf, sizeof(linebuf)); + } + if (res < 0) { + free_if(dynbuf); + return -1; } - return _irc_relay_send(channel, modes, relayname, sender, hostsender, line, ircuser, 0, mod); + if (origlen >= sizeof(linebuf) - 50) { + line = dynbuf; + } else { + line = linebuf; + } + bbs_strterm(line, '\r'); + final = line; + } else { + final = msg; } - dup = strdup(msg); - if (ALLOC_FAILURE(dup)) { - return -1; - } - lines = dup; - while ((line = strsep(&lines, "\n"))) { - bbs_strterm(line, '\r'); - if (transform) { - if (transform(line, linebuf, sizeof(linebuf)) < 0) { + linelen = strlen(final); + if (linelen > 510 || (!line && strchr(final, '\r'))) { + /* If has a trailing CR, we need to strip it. + * Since msg is const, we need to use a buffer to modify if that's the case. */ + if (linelen > sizeof(linebuf)) { + dynbuf = strdup(final); + if (ALLOC_FAILURE(dynbuf)) { return -1; } - line = linebuf; + final = line = dynbuf; + } else { + strcpy(linebuf, final); /* Safe */ + final = line = linebuf; + } + } + + /* Truncation becomes likely beyond this point, so split the message up. + * This is because the relay modules may prefix a sender name, + * and the raw IRC message itself prefixes the command name and the channel. + * Thus, we may add something like: + * PRIVMSG #channel : + * and this could be 40-50 bytes, potentially. + * Being conservative on message length here reduces the chance that + * the message will be within bounds within the BBS but exceed 512 when it hits lirc + * to actually generate the final message. */ + while (linelen > 470) { + char c; + /* Message is too long to be a single message towards IRC. + * Split it up. */ + char *end = line + 465; /* Leave room for the relay module to prefix a username later. */ + const char *midpoint = line + 235; + /* Look for a graceful position to split the message (a space). + * In the worst case, there are no spaces, and we should split the + * message in the second half no matter what, so we send a maximum + * of 2-3 messages per ~512 characters. */ +#pragma GCC diagnostic push /* ignore spurious warning */ +#pragma GCC diagnostic ignored "-Wstrict-overflow" + while (end > midpoint && !isspace(*end)) { + end--; } - if (strlen_zero(line)) { - continue; +#pragma GCC diagnostic pop + /* We are guaranteed here that *end is not the last character in the message. */ + end++; /* Move back forward past the space. */ + c = *end; /* Save character at this position. */ + *end = '\0'; /* NUL terminate to split message. */ + _irc_relay_send(channel, modes, relayname, sender, hostsender, final, ircuser, 0, mod); + *end = c; /* Restore character */ + final = line = end; /* This is the beginning of the next chunk to send. */ + linelen = strlen(line); + } + + /* Send entire message, if less than max message length, or the last chunk, if it was larger. */ + res = _irc_relay_send(channel, modes, relayname, sender, hostsender, final, ircuser, 0, mod); + free_if(dynbuf); + return res; +} + +int _irc_relay_send_multiline(const char *channel, enum channel_user_modes modes, const char *relayname, const char *sender, const char *hostsender, const char *msg, int(*transform)(const char *line, char *buf, size_t len), const char *ircuser, void *mod) +{ + int res = 0; + char *dup, *line, *lines; + + if (!strchr(msg, '\n')) { /* Avoid unnecessarily allocating memory if we don't have to. */ + res |= transform_and_send(channel, modes, relayname, sender, hostsender, msg, transform, ircuser, mod); + } else { + dup = strdup(msg); + if (ALLOC_FAILURE(dup)) { + return -1; + } + lines = dup; + while ((line = strsep(&lines, "\n"))) { + res |= transform_and_send(channel, modes, relayname, sender, hostsender, line, transform, ircuser, mod); } - _irc_relay_send(channel, CHANNEL_USER_MODE_NONE, relayname, sender, hostsender, line, ircuser, 0, mod); + free(dup); } - free(dup); - return 0; + return res; } /* XXX 100% horrible horrible (hopefully temporary) kludge - a total lock hack: In this case, it's to emulate recursive locking for this thread stack: