MQTT-C Security Issue Report (mqtt_unpack_publish_response)

[GANGE666]

Overview

An issue was discovered in MQTT-C through 1.1.5. The MQTT input data processing function mqtt_unpack_publish_response in mqtt.c does not validate the length of incoming topic_name_size, which leads to an out-of-bounds read when subsequent processing of the input data. And this could also lead to an integer overflow when calculating the remaining length of incoming response. Eventually causing Denial-of-Service or an information leak, even remote code execution.

Description

In mqtt_unpack_publish_response, topic_name_size is unpack from input data directly (Line 1352). And then buf pointer add topic_name_size without checking if it exceeds the range of buf, which leads to a buffer overflow. ([Line 1355])

And if attacker provide a topic_name_size is bigger than remaining_length, which could leads to an integer overflow. ([Line 1365] and [Line 1367])

ssize_t mqtt_unpack_publish_response(struct mqtt_response *mqtt_response, const uint8_t *buf)
{
...
        /* parse variable header */
    response->topic_name_size = __mqtt_unpack_uint16(buf);
    buf += 2;
    response->topic_name = buf;
    buf += response->topic_name_size;                           // buffer overflow

if (response->qos_level > 0) {
        response->packet_id = __mqtt_unpack_uint16(buf);
        buf += 2;
    }

    /* get payload */
    response->application_message = buf;
    if (response->qos_level == 0) {
        response->application_message_size = fixed_header->remaining_length - response->topic_name_size - 2;  // integer overflow                
    } else {
        response->application_message_size = fixed_header->remaining_length - response->topic_name_size - 4;  // integer overflow
    }
    buf += response->application_message_size;
...
}

https://github.com/LiamBindle/MQTT-C/blob/be12c343ac5b7125d5e15cb9ab2d743de7f4fab4/src/mqtt.c#L1332-L1373

Impact

Denial-of-Service or an information leak, even remote code execution.

[shayneoneill]

Is this fixed in any forks? Its a little disturbing this has sat around since febuary not even acknowledged.

For all we know this could have been actively exploited for the last 8+ months without even a "We'll look into it".

Does anyone have any recommendations for an alternative library for embedded microcontrollers that is, in fact, maintained?

[LiamBindle]

Happy to accept a PR fixing this. I maintain this repo casually and I'm not paid; MQTT-C in it's current form is stable and pretty widely used. It looks like the author described how to fix the problem---not sure why this wasn't submitted as a PR, but I'm happy to merge a fix.