From 424215f4bd35425822605d8f4f83f58aaadc52d6 Mon Sep 17 00:00:00 2001 From: ajit bansal Date: Thu, 12 Aug 2021 16:26:28 +0530 Subject: [PATCH] Issue:With MQTTCLIENT_PERSISTENCE_DEFAULT the paho code crashes for MQTTVERSION_5 #1108 For MQTTVERSION_5 for SUBSCRIBE command qoss details was not added in persistent folder. Also at time of retrieving for SUBSCRIBE & UNSUBSCRIBE command ptr value was not update properly for MQTTProperties value in the buffer. Because of which MQTTProperties_read() would not return 1 and qcommand ends being free and later down the call stack crash would happen. This fix covers above two issue. --- src/MQTTAsyncUtils.c | 51 +++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/src/MQTTAsyncUtils.c b/src/MQTTAsyncUtils.c index 8c4554ec..41ad44c5 100644 --- a/src/MQTTAsyncUtils.c +++ b/src/MQTTAsyncUtils.c @@ -254,13 +254,15 @@ static int MQTTAsync_persistCommand(MQTTAsync_queuedCommand* qcmd) int chars = 0; /* number of chars from snprintf */ int props_allocated = 0; int process = 1; + int multiplier = 2; /* default value 2 for MQTTVERSION < 5 */ FUNC_ENTRY; switch (command->type) { case SUBSCRIBE: + multiplier = (aclient->c->MQTTVersion >= MQTTVERSION_5) ? 3 : 2; nbufs = ((aclient->c->MQTTVersion >= MQTTVERSION_5) ? 4 : 3) + - (command->details.sub.count * 2); + (command->details.sub.count * multiplier); if (((lens = (int*)malloc(nbufs * sizeof(int))) == NULL) || ((bufs = malloc(nbufs * sizeof(char *))) == NULL)) @@ -282,12 +284,9 @@ static int MQTTAsync_persistCommand(MQTTAsync_queuedCommand* qcmd) bufs[bufindex] = command->details.sub.topics[i]; lens[bufindex++] = (int)strlen(command->details.sub.topics[i]) + 1; - if (aclient->c->MQTTVersion < MQTTVERSION_5) - { - bufs[bufindex] = &command->details.sub.qoss[i]; - lens[bufindex++] = sizeof(command->details.sub.qoss[i]); - } - else + bufs[bufindex] = &command->details.sub.qoss[i]; + lens[bufindex++] = sizeof(command->details.sub.qoss[i]); + if (aclient->c->MQTTVersion >= MQTTVERSION_5) { if (command->details.sub.count == 1) { @@ -456,7 +455,7 @@ static MQTTAsync_queuedCommand* MQTTAsync_restoreCommand(char* buffer, int bufle switch (command->type) { case SUBSCRIBE: - if (qcommand->not_restored == 0) + if (qcommand->not_restored == 1) break; if (&ptr[sizeof(int)] > endpos) goto error_exit; @@ -467,16 +466,17 @@ static MQTTAsync_queuedCommand* MQTTAsync_restoreCommand(char* buffer, int bufle { if ((command->details.sub.topics = (char **)malloc(sizeof(char *) * command->details.sub.count)) == NULL) goto error_exit; - if (MQTTVersion < MQTTVERSION_5) + if ((command->details.sub.qoss = (int *)malloc(sizeof(int) * command->details.sub.count)) == NULL) + goto error_exit; + + if ((MQTTVersion >= MQTTVERSION_5)) { - if ((command->details.sub.qoss = (int *)malloc(sizeof(int) * command->details.sub.count)) == NULL) - goto error_exit; - } - else if (command->details.sub.count > 1) - { - command->details.sub.optlist = (MQTTSubscribe_options*)malloc(sizeof(MQTTSubscribe_options) * command->details.sub.count); - if (command->details.sub.optlist == NULL) - goto error_exit; + if (command->details.sub.count > 1) + { + command->details.sub.optlist = (MQTTSubscribe_options*)malloc(sizeof(MQTTSubscribe_options) * command->details.sub.count); + if (command->details.sub.optlist == NULL) + goto error_exit; + } } } @@ -488,18 +488,15 @@ static MQTTAsync_queuedCommand* MQTTAsync_restoreCommand(char* buffer, int bufle if ((command->details.sub.topics[i] = malloc(data_size)) == NULL) goto error_exit; - strcpy(command->details.sub.topics[i], ptr); ptr += data_size; - if (MQTTVersion < MQTTVERSION_5) - { - if (&ptr[sizeof(int)] > endpos) - goto error_exit; - command->details.sub.qoss[i] = *(int*)ptr; - ptr += sizeof(int); - } - else + if (&ptr[sizeof(int)] > endpos) + goto error_exit; + command->details.sub.qoss[i] = *(int*)ptr; + ptr += sizeof(int); + + if (MQTTVersion >= MQTTVERSION_5) { if (&ptr[sizeof(MQTTSubscribe_options)] > endpos) goto error_exit; @@ -518,7 +515,7 @@ static MQTTAsync_queuedCommand* MQTTAsync_restoreCommand(char* buffer, int bufle break; case UNSUBSCRIBE: - if (qcommand->not_restored == 0) + if (qcommand->not_restored == 1) break; if (&ptr[sizeof(int)] > endpos)