Skip to content

Fix Unbounded sprintf with sender_name in BaseChatMesh.cpp#1975

Open
hylaride wants to merge 1 commit intomeshcore-dev:devfrom
hylaride:dev
Open

Fix Unbounded sprintf with sender_name in BaseChatMesh.cpp#1975
hylaride wants to merge 1 commit intomeshcore-dev:devfrom
hylaride:dev

Conversation

@hylaride
Copy link

@hylaride hylaride commented Mar 8, 2026

sprintf((char *)&temp[5], "%s: ", sender_name) has no length limit. If sender_name is longer than the space left in temp (about MAX_TEXT_LEN + 32 bytes from temp[5] onward), the write goes past the end of temp.

A remote host (e.g. over BLE or other future code) could send a long string as sender_name and trigger the overflow when the device sends a group message.

This fix updates src/helpers/BaseChatMesh.cpp (sendGroupMessage):

  1. Bounded length – The prefix ("sender: ") is written with snprintf using prefix_buf_sz = MAX_TEXT_LEN + 32, so output is limited to the space in temp after the 5-byte header and no overflow is possible.
  2. Null sender – If sender_name is null, sender_name ? sender_name : "" is used so the format string always gets a valid string and you avoid undefined behavior.
  3. Guaranteed termination – ((char *)&temp[5])[prefix_buf_sz - 1] = '\0' forces a null terminator at the end of the prefix buffer so strchr and later uses always see a valid C string.
  4. Type of prefix_len – The result of ep - (char *)&temp[5] is cast to (int) so it matches the existing prefix_len type.

Currently, the companion radio example in examples/companion_radio/MyMesh.cpp already bounds name:

int nlen = len - 1;
if (nlen > sizeof(_prefs.node_name) - 1) nlen = sizeof(_prefs.node_name) - 1;
memcpy(_prefs.node_name, &cmd_frame[1], nlen);
_prefs.node_name[nlen] = 0;

so most users shouldn't currently experience this, but who knows what future people may do with their own clients.

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.

1 participant