Buffer overflow in ws_read()

[ghost]

A buffer overflow occurs when a websocket frame is bigger then the receive buffer is received.

len holds the read buffer length. rlen represents the number of bytes read and should be equal or smaller then len. payload_len is extracted out of the received data and is used to process the received data.

However payload_len is never checked against rlen. This results into a buffer overflow when a websocket frame bigger then the receive buffer is received.

[david-cermak]

Hi Oscar,

it was already fixed on an idf branch, but will be merged to master once IDFv3.2 is released.

Latest IDF already refers to the fixed esp-mqtt, so if you can please check the esp-idf master (no need to add esp-mqtt) and see the issue fixed, might you help closing this issue, please?

Thank you,
David

[ghost]

Hi David,

I'm no longer working on the project that encountered this bug, but I will try to persuade my colleague to test the fix.

Is there an estimated release date for v3.2?

Kind regards,
Oscar

[david-cermak]

Hi, Not a planned release date for IDFv3.2 yet, but we can expect somewhere in late November.

Thanks for mentioning to your colleagues 👍

Regards,
David

[jejer]

Hi guys,
Please check rlen and enhance entire ws_read().
I could not see the fix in esp-idf master branch, could you point out which branch included the fix?

[david-cermak]

Hi @jejer

This fix is still in esp internal repo, but updated the commit message so will get notified once it's merged and published to GitHub

The fix I was talking about in October addressed only longer messages (this is handed correctly in mqtt_client.c when reading payload only by underlying transport -- tcp for ws). however there's still an issue if multiple shorter websocket messages reside in one read buffer and that must be handled in idf in ws_read()

[jejer]

Hi @david-cermak
Thanks for your info.
I have written a private ws_client according to the existing one for my project.
Included the rlen check and read frame by frame.