Skip to content

lose null pointer check in isTopicMatched() inMQTT #11802

@TheSilentDawn

Description

@TheSilentDawn

Description of defect

URL: https://os.mbed.com/teams/mqtt/
Upstream URL: https://github.com/eclipse/paho.mqtt.embedded-c
Type: DoS

The MQTT library is used to receive, parse and send mqtt packet between a broker and a client. The function readMQTTLenString() is called by the function MQTTDeserialize_publish() to get the length and content of the MQTT topic name. It parses the MQTT input linearly. Once a type-length-value tuple is parsed, the index is increased correspondingly. The maximum index is restricted by the length of the received packet size, as shown in line 4 of the code snippet below.

int readMQTTLenString(MQTTString* mqttstring, unsigned char** pptr, unsigned char* enddata)
{
    ...
    if (&(*pptr)[mqttstring->lenstring.len] <= enddata)
    {
        mqttstring->lenstring.data = (char*)*pptr;
        *pptr += mqttstring->lenstring.len;
        rc = 1;
    }
    ...
}

Note that mqttstring->lenstring.len is a part of user input, which can be manipulated. An attacker can simply change it to a larger value to invalidate the if statement so that the statements from line 6 to 8 are skipped, leaving the value of mqttstring->lenstring.data default to zero. Later, the value of mqttstring->lenstring.data is assigned to curn (line 4 of the code snippet below), which is zero under the attack. In line 9, *curn is accessed. In an ARM cortex-M chip, the value at address 0x0 is actually the initialization value for the MSP register. It is highly dependent on the actual firmware. Therefore, the behavior of the program is unpredictable from this time on.

bool MQTT::Client<Network, Timer, a, b>::isTopicMatched(char* topicFilter, MQTTString& topicName)
{
	char* curf = topicFilter;
	char* curn = topicName.lenstring.data;
	char* curn_end = curn + topicName.lenstring.len;

	while (*curf && curn < curn_end)
	{
		if (*curn == '/' && *curf != '/')
		  	break;
		if (*curf != '+' && *curf != '#' && *curf != *curn)
		  	break;
		if (*curf == '+')
		{   // skip until we meet the next separator, or end of string
		  	char* nextpos = curn + 1;
		  	while (nextpos < curn_end && *nextpos != '/')
		      	nextpos = ++curn + 1;
		}
		else if (*curf == '#')
		   	curn = curn_end - 1;    // skip until end of string
		curf++;
		curn++;
	};

	return (curn == curn_end) && (*curf == '\0');
}

Result: A malformed MQTT packet may cause unexpected behaviors depending on the value stored at the address zero on the board.

Target(s) affected by this defect ?

MQTT library of MbedOS

Toolchain(s) (name and version) displaying this defect ?

N/A

What version of Mbed-os are you using (tag or sha) ?

Mbed OS MQTT library 02 Nov 2017
Mbed OS latest release

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

mbed-cli latest version

How is this defect reproduced ?

Use bug_mqtt_1 and bug_mqtt_2 as received mqtt packet after mqtt connect and subscribe
bug_mqtt_1.log
bug_mqtt_2.log

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions