Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MQTTPacket_decodenb() failure mode #57

Closed
scaprile opened this issue Nov 29, 2016 · 2 comments
Closed

MQTTPacket_decodenb() failure mode #57

scaprile opened this issue Nov 29, 2016 · 2 comments
Assignees
Labels
Milestone

Comments

@scaprile
Copy link
Contributor

Hi everyone, hi Ian
I happened to have found a bug in my MQTTPacket.c non-blocking
functions, particularly the one that extracts the message length:
MQTTPacket_decodenb()

On really slow links, if TCP (or whatever underlying transport) fails to
provide the whole length bytes before this function is called 4 times,
next call will exit with an error code. The problem is that the function
is also counting "additional entries" in addition to properly counting
received bytes.
Correction is attached: correctdecodenb.zip
Basically:

diff --git a/MQTTPacket/src/MQTTPacket.c b/MQTTPacket/src/MQTTPacket.c
index bd5f90a..e4746cd 100644
--- a/MQTTPacket/src/MQTTPacket.c
+++ b/MQTTPacket/src/MQTTPacket.c
@@ -333,14 +333,15 @@ static int MQTTPacket_decodenb(MQTTTransport *trp)
 	}
 	do {
 		int frc;
-		if (++(trp->len) > MAX_NO_OF_REMAINING_LENGTH_BYTES)
+		if (trp->len >= MAX_NO_OF_REMAINING_LENGTH_BYTES)
 			goto exit;
 		if ((frc=(*trp->getfn)(trp->sck, &c, 1)) == -1)
 			goto exit;
 		if (frc == 0){
 			rc = 0;
 			goto exit;
-		}
+		}
+		++(trp->len);
 		trp->rem_len += (c & 127) * trp->multiplier;
 		trp->multiplier *= 128;
 	} while ((c & 128) != 0);

PS: This bug hunting has been done as part of a project ending nov 30th, which will also provide new samples and a serial transport to Paho MQTT. See you later guys.

@scaprile
Copy link
Contributor Author

See also #16

@scaprile scaprile mentioned this issue Jul 5, 2017
icraggs added a commit that referenced this issue Jul 7, 2017
@icraggs
Copy link
Contributor

icraggs commented Jul 7, 2017

Merged the PR. Thanks!

@icraggs icraggs closed this as completed Jul 7, 2017
@icraggs icraggs added this to the 1.1 milestone Jul 7, 2017
@icraggs icraggs added the bug label Jul 7, 2017
@icraggs icraggs self-assigned this Jul 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants