Robustness Improvements by MarcAntoineCRUE · Pull Request #91 · hmueller01/pubsubclient3

MarcAntoineCRUE · 2026-02-25T20:47:17Z

Summary

This merge request significantly improves the robustness and safety of the PubSubClient library, without changing its public API or breaking existing usage. All changes are fully covered by the test suite (57/57 tests passing).

Key Fixes

Payload Length Underflow Protection: In handlePacket(), the calculation of payloadLen is now guarded to prevent unsigned underflow if a malformed packet claims a topic length larger than the actual buffer. This prevents potential buffer overreads and undefined behavior.
Payload Offset Overflow Protection: The calculation of payloadOffset now uses size_t instead of uint16_t to avoid silent overflow when handling large topics.
Consistent State in setBufferSize():
The buffer size and pointer are only updated after a successful allocation. If allocation fails, the previous buffer and size remain unchanged, preventing inconsistent internal state.
Null Buffer Guards in connect() and disconnect(): Both methods now check for a valid buffer pointer before use, preventing possible null pointer dereference if buffer allocation failed at construction.
Type Safety in subscribeImpl() and unsubscribeImpl(): Internal length variables now use size_t instead of uint16_t to avoid narrowing/truncation when handling large topics.

Rationale

These changes address potential edge cases that could lead to:

Buffer overflows or underflows
Crashes due to null pointer dereference
Silent data corruption with large topics or payloads
Inconsistent internal state after memory allocation failures

All fixes are implemented with minimal code changes and maintain full backward compatibility.

…WritePos type for better compatibility

…n PubSubClient

TD-er · 2026-02-25T21:24:09Z

src/PubSubClient.cpp

                    return false;
                }
+                size_t payloadLen = length - payloadOffset;  // safe: payloadOffset <= length guaranteed above
+                uint8_t* payload = _buffer + payloadOffset;


Hmm a bit hard to see by just browsing the code in the browser on GH, but shouldn't here also be some check against _bufferSize

You are right, I forgot some checks

No, not at that location handlePacket() can't be called without readPacket(). But I would suggest checking in loop(), with that nothing will ever be read.

I proposed a solution, could you check if it is ok for you?

hmueller01

A first glimps over the changes ... lots of good stuff, but still a few changes.

src/PubSubClient.cpp

src/PubSubClient.h

…h and message ID in PubSubClient

…tibility

…re adequate buffer size for MQTT packets

…ing buffer safety checks and memory management improvements

…and source files

…ion for better clarityCorrection in changelog

MarcAntoineCRUE · 2026-02-26T13:01:16Z

@hmueller01 I think I finished the modification and ready for your final review if I'm right

github-actions · 2026-02-26T13:37:33Z

Memory usage change @ 2e44c48

Board	flash	%	RAM for global variables	%
`arduino:avr:uno`	🔺 +296 - +430	+0.92 - +1.33	0 - 0	0.0 - 0.0
`arduino:megaavr:uno2018`	🔺 +124 - +254	+0.25 - +0.52	0 - 0	0.0 - 0.0
`arduino:samd:mkr1000`	🔺 +72 - +72	+0.03 - +0.03	0 - 0	0.0 - 0.0
`esp32:esp32:esp32`	🔺 +52 - +60	0.0 - 0.0	0 - 0	0.0 - 0.0
`esp8266:esp8266:generic`	N/A	N/A	N/A	N/A

Click for full report table

Board	`examples/mqtt_auth` flash	%	`examples/mqtt_auth` RAM for global variables	%	`examples/mqtt_basic` flash	%	`examples/mqtt_basic` RAM for global variables	%	`examples/mqtt_progmem` flash	%	`examples/mqtt_progmem` RAM for global variables	%	`examples/mqtt_publish_in_callback` flash	%	`examples/mqtt_publish_in_callback` RAM for global variables	%	`examples/mqtt_reconnect_nonblocking` flash	%	`examples/mqtt_reconnect_nonblocking` RAM for global variables	%	`examples/mqtt_stream` flash	%	`examples/mqtt_stream` RAM for global variables	%	`examples/mqtt_esp` flash	%	`examples/mqtt_esp` RAM for global variables	%	`examples/mqtt_esp_large_message` flash	%	`examples/mqtt_esp_large_message` RAM for global variables	%
`arduino:avr:uno`	296	0.92	0	0.0	430	1.33	0	0.0	426	1.32	0	0.0	296	0.92	0	0.0	386	1.2	0	0.0	304	0.94	0	0.0
`arduino:megaavr:uno2018`	130	0.27	0	0.0	124	0.25	0	0.0	254	0.52	0	0.0	130	0.27	0	0.0	128	0.26	0	0.0	130	0.27	0	0.0
`arduino:samd:mkr1000`	72	0.03	0	0.0	72	0.03	0	0.0	72	0.03	0	0.0	72	0.03	0	0.0	72	0.03	0	0.0	72	0.03	0	0.0
`esp32:esp32:esp32`	52	0.0	0	0.0	52	0.0	0	0.0	52	0.0	0	0.0	52	0.0	0	0.0	52	0.0	0	0.0					56	0.0	0	0.0	60	0.0	0	0.0
`esp8266:esp8266:generic`	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A

Click for full report CSV

Board,examples/mqtt_auth<br>flash,%,examples/mqtt_auth<br>RAM for global variables,%,examples/mqtt_basic<br>flash,%,examples/mqtt_basic<br>RAM for global variables,%,examples/mqtt_progmem<br>flash,%,examples/mqtt_progmem<br>RAM for global variables,%,examples/mqtt_publish_in_callback<br>flash,%,examples/mqtt_publish_in_callback<br>RAM for global variables,%,examples/mqtt_reconnect_nonblocking<br>flash,%,examples/mqtt_reconnect_nonblocking<br>RAM for global variables,%,examples/mqtt_stream<br>flash,%,examples/mqtt_stream<br>RAM for global variables,%,examples/mqtt_esp<br>flash,%,examples/mqtt_esp<br>RAM for global variables,%,examples/mqtt_esp_large_message<br>flash,%,examples/mqtt_esp_large_message<br>RAM for global variables,%
arduino:avr:uno,296,0.92,0,0.0,430,1.33,0,0.0,426,1.32,0,0.0,296,0.92,0,0.0,386,1.2,0,0.0,304,0.94,0,0.0
arduino:megaavr:uno2018,130,0.27,0,0.0,124,0.25,0,0.0,254,0.52,0,0.0,130,0.27,0,0.0,128,0.26,0,0.0,130,0.27,0,0.0
arduino:samd:mkr1000,72,0.03,0,0.0,72,0.03,0,0.0,72,0.03,0,0.0,72,0.03,0,0.0,72,0.03,0,0.0,72,0.03,0,0.0
esp32:esp32:esp32,52,0.0,0,0.0,52,0.0,0,0.0,52,0.0,0,0.0,52,0.0,0,0.0,52,0.0,0,0.0,,,,,56,0.0,0,0.0,60,0.0,0,0.0
esp8266:esp8266:generic,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A

src/PubSubClient.cpp

hmueller01

I proposed a solution, could you check if it is ok for you?

The change looks good for me!

…erSize methods

…te method

github-actions · 2026-02-27T11:45:46Z

Memory usage change @ 433923c

Board	flash	%	RAM for global variables	%
`arduino:avr:uno`	🔺 +336 - +534	+1.04 - +1.66	0 - 0	0.0 - 0.0
`arduino:megaavr:uno2018`	🔺 +164 - +362	+0.34 - +0.74	0 - 0	0.0 - 0.0
`arduino:samd:mkr1000`	🔺 +96 - +96	+0.04 - +0.04	0 - 0	0.0 - 0.0
`esp32:esp32:esp32`	🔺 +80 - +92	+0.01 - +0.01	0 - 0	0.0 - 0.0
`esp8266:esp8266:generic`	N/A	N/A	N/A	N/A

Click for full report table

Board	`examples/mqtt_auth` flash	%	`examples/mqtt_auth` RAM for global variables	%	`examples/mqtt_basic` flash	%	`examples/mqtt_basic` RAM for global variables	%	`examples/mqtt_progmem` flash	%	`examples/mqtt_progmem` RAM for global variables	%	`examples/mqtt_publish_in_callback` flash	%	`examples/mqtt_publish_in_callback` RAM for global variables	%	`examples/mqtt_reconnect_nonblocking` flash	%	`examples/mqtt_reconnect_nonblocking` RAM for global variables	%	`examples/mqtt_stream` flash	%	`examples/mqtt_stream` RAM for global variables	%	`examples/mqtt_esp` flash	%	`examples/mqtt_esp` RAM for global variables	%	`examples/mqtt_esp_large_message` flash	%	`examples/mqtt_esp_large_message` RAM for global variables	%
`arduino:avr:uno`	336	1.04	0	0.0	470	1.46	0	0.0	534	1.66	0	0.0	336	1.04	0	0.0	426	1.32	0	0.0	344	1.07	0	0.0
`arduino:megaavr:uno2018`	170	0.35	0	0.0	164	0.34	0	0.0	362	0.74	0	0.0	170	0.35	0	0.0	168	0.35	0	0.0	170	0.35	0	0.0
`arduino:samd:mkr1000`	96	0.04	0	0.0	96	0.04	0	0.0	96	0.04	0	0.0	96	0.04	0	0.0	96	0.04	0	0.0	96	0.04	0	0.0
`esp32:esp32:esp32`	80	0.01	0	0.0	80	0.01	0	0.0	80	0.01	0	0.0	80	0.01	0	0.0	80	0.01	0	0.0					84	0.01	0	0.0	92	0.01	0	0.0
`esp8266:esp8266:generic`	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A

Click for full report CSV

Board,examples/mqtt_auth<br>flash,%,examples/mqtt_auth<br>RAM for global variables,%,examples/mqtt_basic<br>flash,%,examples/mqtt_basic<br>RAM for global variables,%,examples/mqtt_progmem<br>flash,%,examples/mqtt_progmem<br>RAM for global variables,%,examples/mqtt_publish_in_callback<br>flash,%,examples/mqtt_publish_in_callback<br>RAM for global variables,%,examples/mqtt_reconnect_nonblocking<br>flash,%,examples/mqtt_reconnect_nonblocking<br>RAM for global variables,%,examples/mqtt_stream<br>flash,%,examples/mqtt_stream<br>RAM for global variables,%,examples/mqtt_esp<br>flash,%,examples/mqtt_esp<br>RAM for global variables,%,examples/mqtt_esp_large_message<br>flash,%,examples/mqtt_esp_large_message<br>RAM for global variables,%
arduino:avr:uno,336,1.04,0,0.0,470,1.46,0,0.0,534,1.66,0,0.0,336,1.04,0,0.0,426,1.32,0,0.0,344,1.07,0,0.0
arduino:megaavr:uno2018,170,0.35,0,0.0,164,0.34,0,0.0,362,0.74,0,0.0,170,0.35,0,0.0,168,0.35,0,0.0,170,0.35,0,0.0
arduino:samd:mkr1000,96,0.04,0,0.0,96,0.04,0,0.0,96,0.04,0,0.0,96,0.04,0,0.0,96,0.04,0,0.0,96,0.04,0,0.0
esp32:esp32:esp32,80,0.01,0,0.0,80,0.01,0,0.0,80,0.01,0,0.0,80,0.01,0,0.0,80,0.01,0,0.0,,,,,84,0.01,0,0.0,92,0.01,0,0.0
esp8266:esp8266:generic,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A

hmueller01 · 2026-02-27T16:00:15Z

src/PubSubClient.cpp

-        } else {
-            return false;
-        }
+        newBuffer = (uint8_t*)realloc(_buffer, size);


I was not sure, if the original buffer is still intact, if realloc fails. So I read the man ...

If realloc() fails the original block is left untouched; it is not freed or moved.

hmueller01 · 2026-02-27T19:31:54Z

@MarcAntoineCRUE Somehow endPublish() fails now ...

MarcAntoineCRUE added 2 commits February 25, 2026 21:36

Refactor write methods for improved buffer handling and update buffer…

4f5ae2d

…WritePos type for better compatibility

Enhance buffer safety checks and improve memory allocation handling i…

5cf5dd6

…n PubSubClient

TD-er reviewed Feb 25, 2026

View reviewed changes

hmueller01 requested changes Feb 25, 2026

View reviewed changes

src/PubSubClient.cpp Outdated Show resolved Hide resolved

src/PubSubClient.cpp Outdated Show resolved Hide resolved

src/PubSubClient.cpp Outdated Show resolved Hide resolved

src/PubSubClient.cpp Outdated Show resolved Hide resolved

src/PubSubClient.h Outdated Show resolved Hide resolved

MarcAntoineCRUE added 3 commits February 25, 2026 22:45

Improve packet handling with additional safety checks for topic lengt…

579e662

…h and message ID in PubSubClient

Change bufferWritePos type from uint16_t to size_t for improved compa…

6e9b2a0

…tibility

Enhance buffer safety checks in handlePacket and loop methods to ensu…

a570902

…re adequate buffer size for MQTT packets

MarcAntoineCRUE requested a review from hmueller01 February 25, 2026 22:02

Update CHANGELOG.md to document recent enhancements and fixes, includ…

a9abbe0

…ing buffer safety checks and memory management improvements

MarcAntoineCRUE force-pushed the dev/performances branch from c3c8dc0 to a9abbe0 Compare February 25, 2026 22:04

Merge remote-tracking branch 'pubsub3/master' into dev/performances

ccb047d

MarcAntoineCRUE force-pushed the dev/performances branch from 0178b46 to ccb047d Compare February 26, 2026 12:48

MarcAntoineCRUE added 2 commits February 26, 2026 13:49

Refactor comments for clarity and consistency in PubSubClient header …

919e96c

…and source files

Reorganize CHANGELOG.md to move QoS improvements under "Changed" sect…

2e44c48

…ion for better clarityCorrection in changelog

hmueller01 requested changes Feb 26, 2026

View reviewed changes

hmueller01 reviewed Feb 26, 2026

View reviewed changes

MarcAntoineCRUE added 3 commits February 27, 2026 12:18

Improve readability of conditional checks in handlePacket and setBuff…

12e0457

…erSize methods

Refactor write methods to improve buffer handling and reduce redundancy

18845e4

Remove unused algorithm include and simplify chunk calculation in wri…

433923c

…te method

hmueller01 reviewed Feb 27, 2026

View reviewed changes

Conversation

MarcAntoineCRUE commented Feb 25, 2026

Summary

Key Fixes

Rationale

Uh oh!

TD-er Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

MarcAntoineCRUE Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

hmueller01 Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

MarcAntoineCRUE Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

hmueller01 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MarcAntoineCRUE commented Feb 26, 2026

Uh oh!

github-actions bot commented Feb 26, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hmueller01 left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 27, 2026

Uh oh!

hmueller01 Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

hmueller01 commented Feb 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants