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

C client's readPacket can overflow read buffer #40

Closed
cpacey opened this issue Jun 8, 2016 · 4 comments
Closed

C client's readPacket can overflow read buffer #40

cpacey opened this issue Jun 8, 2016 · 4 comments
Assignees
Labels
Milestone

Comments

@cpacey
Copy link

cpacey commented Jun 8, 2016

The C client's readPacket reads the message size from the incoming packet and then reads that much data into the read buffer (c->readbuf) with no regard for the read buffer's size (c->readbuf_size). This means that messages larger than the read buffer's size will always cause a buffer overflow.

We're using FreeRTOS, with a read buffer that's allocated on the stack of the task that's handling Mqtt. When this buffer overflow occurs it smashes our stack (including the MQTTClient) and leads to anomalous behaviour.

@andrewchambers
Copy link

andrewchambers commented Jul 3, 2016

Yes, this library seems to have many buffer overflows and other similar issues, I would not recommend using it when you don't trust your endpoint.

@eden-wang
Copy link

I found the problem too. When read buffer is not enough, I disgard the rest data. Here is the main codes

/* erase useless data */
read_bytes = 0;
while (read_bytes < rem_len || expired(timer))
{
    read_bytes += c->ipstack->mqttread(c->ipstack,
            c->readbuf,
            min(c->readbuf_size, rem_len - read_bytes),
            left_ms(timer));
}

@icraggs
Copy link
Contributor

icraggs commented Jul 7, 2017

I've added a fix for this in readPacket which I believe does most of the job. If the read buffer is very small (less than the header + length of remaining length, between 0 and 5) then that could still be a problem, but buffers of that size are not very useful anyway.

@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
@icraggs icraggs closed this as completed Jul 7, 2017
@AkremToujani
Copy link

Hi Teams,
I'm using paho mqtt client with NUCLEO STM32 board,
I fixed the KeepAlive Interval to 30 min (1800s) and yield(100), but, after 9 hours the client has disconnected.

any help please
thanks a lot

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

5 participants