Skip to content

Commit

Permalink
net_irc: Fix truncation of relayed messages.
Browse files Browse the repository at this point in the history 
This improves the IRC message relay process to ensure
that long messages are properly split into multiple
messages if needed to send successfully to IRC.

We also now do more to prevent message truncation
and actively warn about truncation if we detect it.

This address the miscellaneous truncation of various
messages that previously could occur silently without warning.
  • Loading branch information
@InterLinked1
InterLinked1 committed Dec 1, 2023
1 parent 203096e commit de10464
Show file tree
Hide file tree
Showing 3 changed files with 145 additions and 40 deletions.
28 changes: 17 additions & 11 deletions modules/mod_irc_client.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
Expand Down Expand Up @@ -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;

Expand All @@ -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;
}
Expand All @@ -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;

Expand All @@ -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;
}
Expand All @@ -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;
}

Expand Down
8 changes: 6 additions & 2 deletions modules/mod_irc_relay.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
149 changes: 122 additions & 27 deletions nets/net_irc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 :<username>
* 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:
Expand Down

0 comments on commit de10464

Please sign in to comment.