Skip to content

bound remain_len in publishresp, disconnect and auth decoders#553

Closed
jmestwa-coder wants to merge 1 commit into
wolfSSL:masterfrom
jmestwa-coder:decoder-remain-len-bound
Closed

bound remain_len in publishresp, disconnect and auth decoders#553
jmestwa-coder wants to merge 1 commit into
wolfSSL:masterfrom
jmestwa-coder:decoder-remain-len-bound

Conversation

@jmestwa-coder

Copy link
Copy Markdown

reason code byte read past rx_buf in three v5 ack decoders:

  • MqttDecode_PublishResp reads the reason code gated on remain_len, not rx_buf_len
  • MqttDecode_Disconnect and MqttDecode_Auth do the same one byte after the fixed header
  • a frame claiming a reason code but ending at the header steps one byte past the caller buffer

added the rx_buf_len < header_len + remain_len guard that MqttDecode_Connect and MqttDecode_Subscribe already carry, plus regression tests that fault pre-patch and return out-of-buffer after.

@wolfSSL-Bot

Copy link
Copy Markdown

Can one of the admins verify this patch?

@embhorn

embhorn commented Jun 15, 2026

Copy link
Copy Markdown
Member

Hi @jmestwa-coder

Thanks for contacting wolfSSL with this fix. We do not typically accept small fixes from new contributors. I will request these to be addressed in #552

Kind regards,
@embhorn - wolfSSL Support

@jmestwa-coder

Copy link
Copy Markdown
Author

No problem, makes sense to roll these into #552. For reference the guards go right after MqttDecode_FixedHeader in MqttDecode_PublishResp, MqttDecode_Disconnect and MqttDecode_Auth, mirroring the same check MqttDecode_Connect and MqttDecode_Subscribe already carry. There are three failing regression tests in tests/test_mqtt_packet.c covering each one if they're useful to pull across. Happy for you to close this out.

@embhorn

embhorn commented Jun 16, 2026

Copy link
Copy Markdown
Member

Closing this PR as it was superseded by #552

@embhorn embhorn closed this Jun 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants