Skip to content

Fix Unbounded sprintf with sender_name in BaseChatMesh.cpp#1975

Closed
hylaride wants to merge 1 commit into
meshcore-dev:devfrom
hylaride:dev
Closed

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

Conversation

@hylaride
Copy link
Copy Markdown

@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.

robekl added a commit to robekl/MeshCore that referenced this pull request Mar 23, 2026
Reimplements meshcore-dev/MeshCore PR meshcore-dev#1975 on top of 467959c for the 1.14.1 maintenance branch.

References:
PR: meshcore-dev#1975
snprintf reference: https://en.cppreference.com/w/c/io/fprintf

Rationale:
The original code used sprintf to prepend the sender name to outgoing text messages without an explicit bound, which is unnecessary risk in a message-construction path. This backport switches to bounded snprintf, handles a null sender name defensively, and keeps the existing truncation logic intact. The patch is confined to one formatting site in BaseChatMesh and is appropriate for a maintenance branch as a focused safety fix.
@hylaride hylaride closed this Apr 27, 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.

1 participant