Security report

[Matt Jones]

Hi,

Here is another issue that should be corrected in the libpurple
source.  More to come soon.

Regards,

Matt

msn_message_gen_payload function in libpurple/protocols/msn/Msg.c file.

char *
msn_message_gen_payload(MsnMessage *msg, size_t *ret_size)
{
GList *l;
char *n, *base, *end;
int len;
size_t body_len = 0;
const void *body;

g_return_val_if_fail(msg != NULL, NULL);

len = MSN_BUF_LEN;

base = n = end = g_malloc(len + 1);
end += len;

/* Standard header. */
if (msg->charset == NULL)
{
 g_snprintf(n, len,
      "MIME-Version: 1.0\r\n"
      "Content-Type: %s\r\n",
      msg->content_type);
}
else
{
 g_snprintf(n, len,
      "MIME-Version: 1.0\r\n"
      "Content-Type: %s; charset=%s\r\n",
      msg->content_type, msg->charset);
}

n += strlen(n);

for (l = msg->header_list; l != NULL; l = l->next)
{
 const char *key;
 const char *value;

 key = l->data;
 value = msn_message_get_header_value(msg, key);

 g_snprintf(n, end - n, "%s: %s\r\n", key, value);
 n += strlen(n);
}

n += g_strlcpy(n, "\r\n", end - n); [1]

body = msn_message_get_bin_data(msg, &body_len); [2]

if (body != NULL)
{
 memcpy(n, body, body_len); [3]
 n += body_len;
 *n = '\0'; [4]
}

if (ret_size != NULL)
{
 *ret_size = n - base;

 if (*ret_size > 1664)
  *ret_size = 1664;
}

return base;
}

Description:

The function builds an MSN message including the headers and binary
data body. It begins by setting a static header, then pushes any
optional headers one at a time.

At [1] a CRLF is appended and n is incremented, however g_strlcpy
always returns the size of the source buffer, so this will always
increment n by 2, regardless if there is room for it.

At [2] the body is fetched using msn_message_get_bin_data, which is a
simple function that just returns msg->body and sets body_len to its
length (msg->body_len).

At [3] body is concatenated onto where n is pointing to with no
validation if there's sufficient room for it, which could result in a
heap overflow.

At [4] n is NULL terminated, this could be out-of-bounds due to either
[1] pr [3].

Suggested fix:

Do not increment a pointer using the return value from g_strncpy
without validating there's sufficient room for the data.  Perform a
validation check that the body returned from msn_message_get_bin_data
can be copied to its destination.