8000 fix error in base64EncodedSize() by mjs973 · Pull Request #2160 · ros/ros_comm · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix error in base64EncodedSize() #2160

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

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.

Mike Scheutzow added 2 commits June 18, 2021 11:12
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)

@sloretz
Copy link
Contributor
sloretz commented Apr 25, 2025

Thank you for the PR!

ROS Noetic will reach end-of-life on May 31st, 2025. Every change comes with a risk of introducing regressions, and there isn't much time left to fix them. To make sure this PR doesn't introduce any regressions please:

  • Describe how you tested this change
  • Recruit at least one more person to review this PR and try it out on their system

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.

4 participants
0