Out of bounds write when processing certain long incoming messages

[josephbisch]

On line 1787 of format_event in text.c, a check is performed to guard against a potential buffer overflow on the next few lines. The check attempts to truncate ar so that o doesn't overflow in the strip_color2 or strip_hidden_attribute functions.

Line 1787 and 1788 linked above are the following:

if (strlen (ar) > sizeofo - oi - 4)
	ar[sizeofo - oi - 4] = 0;	/* Avoid buffer overflow */

It should be noted that sizeofo is always 4096. So if a server can craft a message to a hexchat client such that oi is 4096 at line 1787, then the right side of the comparison is -4. I believe since strlen returns an unsigned integer type, that the -4 is converted to a very large positive number via modulo arithmetic. Line 1788 is skipped and hexchat writes past the end of o. This may be detected with Address Sanitizer or may trigger stack smash protection.

Note that because of IRC protocol message length limitations, it is believed that this is only possible when connecting to a server that doesn't adhere to the 510 character limit and it is believed to not be possible for another client to exploit.

It has been verified that the conditions for the oob write can be reached via a crafted INVITE message and a crafted PRIVMSG command, though those may not be the only message types.

[TingPing]

Sounds accurate, could you submit a PR for this?

[josephbisch]

Sure. I don't understand the reason for subtracting four on those lines (i.e. it looks to me like more than would need to be reserved for the newline and null terminator). Just want to understand that before taking an attempt at a PR.

[TingPing]

I'm not sure offhand but the format is documented here: https://github.com/hexchat/hexchat/blob/master/src/common/text.c#L929-L952