Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix error in base64EncodedSize() #2160

Open
wants to merge 2 commits into
base: melodic-devel
Choose a base branch
from

Conversation

mjs973
Copy link

@mjs973 mjs973 commented Jun 18, 2021

I have been debugging an issue where no messages are delivered over UDP Transport. I'm on melodic, ubuntu 18.04, x86_64 cpu. Here is what I found.

The base64 encoder called by XmlRpcValue.cpp may write two newlines to the end of the encoded string, but this is not accounted for in base64EncodedSize(). This results in a buffer overflow, which then causes ROS UDP Transport connection setup to fail.

One case which results in the double newline is when the total length of "node_name + topic_name + msg_type" is 33 bytes.

The base64 encoder called by XmlRpcValue.cpp may write two newlines to the
end of the encoded string, but this is not accounted for in base64EncodedSize().
This results in a buffer overflow, which then causes ROS UDP Transport
connection setup to fail.

One case which results in the double newline is when the total length
of "node_name + topic_name + msg_type" is 33 bytes.

Signed-off-by: Mike Scheutzow <mjs973@hotmail.com>
Signed-off-by: Mike Scheutzow <mjs973@hotmail.com>
@jacobperron
Copy link
Contributor

Thanks for the patch! Could you provide a complete code example that I can run to verify the bug and fix?

Also, could you target the latest development branch (noetic-devel), and then I'll backport the fix to Melodic.

@JavierUbago
Copy link

JavierUbago commented Sep 28, 2023

I've had this same issue in ROS noetic, my question with a code example that you can run is here: https://robotics.stackexchange.com/questions/104346/issue-with-rostopic-name-length-affecting-node-communication)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants