The bug of tunction isTopicMatch()

[TheSilentDawn]

Description of defect

The source bug is reported in ARMmbed/mbed-os#11802.

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.