Jabber OOB Transfer security issue

[Matt Jones]

Hi,

Please see below another issue related to Jabber OOB transfers.

Thanks,

Matt

Summary:

jabber_oob_xfer_read() is susceptible to a memory corruption issue due
to dangerous handling of the Content-Length HTTP header.

static gssize jabber_oob_xfer_read(guchar **buffer, PurpleXfer *xfer) {
    JabberOOBXfer *jox = xfer->data;
    char test[2048];
    char *tmp, *lenstr;
    int len;

    if((len = read(xfer->fd, test, sizeof(test))) > 0) {
        jox->headers = g_string_append_len(jox->headers, test, len);
        if((tmp = strstr(jox->headers->str, "\r\n\r\n"))) {
            *tmp = '\0';
            lenstr = strstr(jox->headers->str, "Content-Length: "); [1]
            if(lenstr) {
                int size;
                sscanf(lenstr, "Content-Length: %d", &size); [2]
                purple_xfer_set_size(xfer, size);
            }
            purple_xfer_set_read_fnc(xfer, NULL);

            tmp += 4;

            *buffer = (unsigned char*) g_strdup(tmp);
            return strlen(tmp);
        }
        return 0;
    } else if (errno != EAGAIN) {
        purple_debug_error("jabber", "Read error on oob xfer!\n");
        purple_xfer_cancel_local(xfer);
    }

    return 0;
}

Issue

The function reads a content-length parameter from the client [1] and
prepares reading data.  The issue is that it's using a signed integer
for the Content-Length header [2], which is a classic issue that has
been present in many HTTP implementations in the past that parse this
particular header.  This dangerous construct commonly leads to memory
corruption as negative content lengths will affect memory related
operations and/or arithmetic.

Recommendation

It is recommended to change size as an unsigned integer and also
validate that it's within a reasonable range.